-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Refactor AWS Kinesis exporter to allow for extended types of encoding #3434
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update the PR title to something more descriptive
Please update the PR title to something more descriptive. "Main" does not tell what it does. |
Sorry @mx-psi , @tigrannajaryan , it completely skip my mind. |
Hey @mx-psi , @tigrannajaryan , Could I kindly ask you to review this please? |
) | ||
|
||
var ( | ||
// ErrUnsupportedEncodedType is used when the encoder type does not the type of encoding |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// ErrUnsupportedEncodedType is used when the encoder type does not the type of encoding | |
// ErrUnsupportedEncodedType is used when the encoder type does not support the type of encoding |
In order to make the transition from only jaeger traces to otlp data being written to kinesis, we've split up our approach and made sure it does not introduce breaking changes.
Adding in tests for the encoder to ensure the expected errors are being raised.
Sorry, need to close this PR to fix up how we are using our fork. |
…released (open-telemetry#3434) Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
…released (#3434) Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
Description: Moving to encoding type model for exporting data to kinesis while maintaining backwards comparability. Ref #1310
Link to tracking Issue: We have split up the work needed for #1305 as per @owais' comments on the PR linked above.
Testing: I have added rather trivial tests to get this shipped and to make sure it doing what it says on the tin (so to speak).
Documentation: No behaviour changes are being made here, just a change in implementation detail. Any follow up PRs that extend the ability of this exporter will joined with documentation.