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

Update the OTLP exporter batching of traces #623

Merged
merged 1 commit into from
Apr 8, 2020

Conversation

MrAlias
Copy link
Contributor

@MrAlias MrAlias commented Apr 7, 2020

Instead of batching based on the Resource pointer which is not guaranteed to uniquely identify common Resources, use the String method which is.

Renames the external test package to be identified as the integration testing. This testing structure (the mock exporter) batches exported resources transmitted from the resource. This is needed as there is no guarantees that a batch will be exported in a single payload, therefore, it is not an ideal place to add resource batch testing for the exporter itself.

Add unit tests to ensure the Exporter is correctly batching spans by resource.

Instead of batching based on the Resource pointer which is not guaranteed
to uniquely identify common Resources, use the `String` method which is.

Renames the external test package to be identified as the integration
testing. This testing structure (the mock exporter) batches exported
resources transmitted from the resource. This is needed as there is no
guarantees that a batch will be exported in a single payload, therefore,
it is not an ideal place to add resource batch testing for the exporter
itself.

Add unit tests to ensure the Exporter is correctly batching spans by
resource.
@MrAlias MrAlias added exporters area:trace Part of OpenTelemetry tracing labels Apr 7, 2020
@MrAlias MrAlias self-assigned this Apr 7, 2020
Copy link
Member

@krnowak krnowak left a comment

Choose a reason for hiding this comment

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

Looks good. This PR just got me thinking that maybe we should add some documentation to core.Key.{String,Bool,Int,…} functions about the convenience functions like key.{String,Bool,Int…} when you are creating both key and a value at the same time.

@rghetia rghetia merged commit 0a9e861 into open-telemetry:master Apr 8, 2020
@MrAlias MrAlias deleted the batch-otlp branch April 8, 2020 23:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:trace Part of OpenTelemetry tracing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants