Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[extension/awsmiddleware] Add ID to context to trace requests. #128

Merged
merged 5 commits into from
Oct 20, 2023

Conversation

jefchien
Copy link

@jefchien jefchien commented Oct 20, 2023

Description: Previously, there was no way to determine if a received request/response were a pair. Adds an ID to the context in the RequestHandler wrappers and retrieves it in the ResponseHandler wrappers. Passes the ID to the handlers so implementations can track the ID.


Other changes

  • Combines RequestHandlers() and ResponseHandlers() functions into a single Handlers() function to make it easier to create handlers that implement both and need to be configured on both.
  • Changes the AWS SDKv1 response handler to ValidateResponse instead of Unmarshal. This is so the response handler will be reached on 4xx/5xx status codes. If there's an error, it will never get to Unmarshal (see aws-sdk-go requests.go). Keep in mind that if retries are enabled, ValidateResponse will be run on each attempt.
  • Fixes the go.mod package name. Should have been amazon-contributing instead of open-telemetry. This broke imports.
	module declares its path as: github.com/open-telemetry/opentelemetry-collector-contrib/extension/awsmiddleware
	        but was required as: github.com/amazon-contributing/opentelemetry-collector-contrib/extension/awsmiddleware
  • Changes config to an ID to match other extension implementation (e.g. StorageID and Getter function). This allows other components to simply add
Middleware *component.ID `mapstructure:"middleware,omitempty"`

to their configurations and use the awsmiddleware.GetConfigurer(host.GetExtensions(), *cfg.Middleware).

  • Adds mock extension/handler/host to make it easier to test in other components.

Link to tracking Issue: N/A

Testing: Added unit tests for the requests. Imported this commit into go.mod without issue.

Documentation: Updated README to match changes.

SaxyPandaBear
SaxyPandaBear previously approved these changes Oct 20, 2023
Copy link

@SaxyPandaBear SaxyPandaBear left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

adam-mateen
adam-mateen previously approved these changes Oct 20, 2023
@jefchien jefchien merged commit a8ff477 into aws-cwa-dev Oct 20, 2023
63 of 70 checks passed
@jefchien jefchien deleted the fix-middleware branch October 20, 2023 16:08
lisguo pushed a commit to lisguo/opentelemetry-collector-contrib that referenced this pull request Oct 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants