chore(observability): add common internal HTTP metrics to AWS sinks#25508
chore(observability): add common internal HTTP metrics to AWS sinks#25508gwenaskell wants to merge 9 commits into
Conversation
gwenaskell
changed the title
gwenaskell
force-pushed
the
yoenn.burban/OPA-5365-add-aws-http-observability
branch
from
3959a94 to
556590d
Compare
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 556590d4b5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
gwenaskell
force-pushed
the
yoenn.burban/OPA-5365-add-aws-http-observability
branch
from
556590d to
aae5c92
Compare
|
Note The following is an automated Codex code review report. Finding (Medium): AWS auth/region HTTP paths are still uninstrumented The observability wrapper is only applied to the final service client connector (
This means HTTP failures from those paths (e.g., a failed IMDS call or STS assume-role timeout) won't emit the new common HTTP metrics/events. Recommended fix: Either factor the wrapper into a shared helper and apply it to all connector construction sites (if the intent is full AWS observability), or add a comment + test explicitly documenting that only service calls are instrumented and the credential/region paths are out of scope. |
Apply AwsHttpClient instrumentation to STS assume-role, profile/SSO file, and IMDS credential providers. Region resolution remains uninstrumented as no region is available at that point.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c186783669
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@pront wired in telemetry for the AWS auth requests too. |
@gwenaskell Thanks! Did you test this end-to-end? There were some interesting finding about double counting bytes. |
3 participants
Summary
Wired common internal HTTP telemetry into the connector used by AWS sinks. This required some modifications to the internal events to provide compatibility with smithy requests and responses.
Vector configuration
How did you test this PR?
Change Type
Is this a breaking change?
Does this PR include user facing changes?
no-changeloglabel to this PR.References
Notes
@vectordotdev/vectorto reach out to us regarding this PR.pre-pushhook, please see this template.make fmtmake check-clippy(if there are failures it's possible some of them can be fixed withmake clippy-fix)make testgit merge origin masterandgit push.Cargo.lock), pleaserun
make build-licensesto regenerate the license inventory and commit the changes (if any). More details on the dd-rust-license-tool.