Update OTelProtoCodec for InstrumentationLibrary to InstrumentationScope rename #2114
Conversation
Signed-off-by: Chase Engelbrecht <engechas@amazon.com>
Signed-off-by: Chase Engelbrecht <engechas@amazon.com>
Signed-off-by: Chase Engelbrecht <engechas@amazon.com>
Codecov Report
@@ Coverage Diff @@
## main #2114 +/- ##
============================================
+ Coverage 93.76% 93.95% +0.19%
- Complexity 1630 1633 +3
============================================
Files 204 204
Lines 4729 4729
Branches 378 378
============================================
+ Hits 4434 4443 +9
+ Misses 207 196 -11
- Partials 88 90 +2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
@dlvenable @chenqi0805 Could you guys take a look at this PR and verify that the behavior is correct for how DataPrepper should handle the OTel model change? |
There was a problem hiding this comment.
Overall looks good to me. Have you tried on the examples/dev/trace-analytics-sample-app by bumping otel-collector?
| @@ -77,6 +78,9 @@ public class OTelProtoCodecTest { | |||
| private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); | |||
| private static final Random RANDOM = new Random(); | |||
| private static final String TEST_REQUEST_JSON_FILE = "test-request.json"; | |||
| private static final String TEST_REQUEST_INSTRUMENTATION_LIBRARY_JSON_FILE = "test-request-instrumentation-library.json"; | |||
| private static final String TEST_REQUEST_BOTH_SPAN_TYPES_JSON_FILE = "test-request-both-span-types.json"; | |||
There was a problem hiding this comment.
Is this mixture a possible payload scenario from otel-collector? If not, TEST_REQUEST_JSON_FILE(with proper renaming) and TEST_REQUEST_INSTRUMENTATION_LIBRARY_JSON_FILE are sufficient for coverage.
There was a problem hiding this comment.
According to OTEL's comment, clients can double publish
There was a problem hiding this comment.
Have you tried on the examples/dev/trace-analytics-sample-app
I tested by spinning up a trace pipeline manually with stdout as the sink and verified the data made it through
There was a problem hiding this comment.
I used tracegen to push load with Otel collector: image: otel/opentelemetry-collector-contrib:latest
There was a problem hiding this comment.
I can confirm that the dev/trace-analytics-sample-app is working now (with additional change to the Data Prepper Version constant). That example also works with a recent OTEL collector version (0.68.0).
Description
The InstrumentationLibrary model was renamed to InstrumentationScope as part of this PR: open-telemetry/opentelemetry-proto#362
This PR updates the
OTelProtoDecoderandOTelProtoEncoderto use the newInstrumentationScopemodel while continuing to support incoming requests usingInstrumentationLibraryper the specification defined in the above PR.Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.