Update otel proto buf specification #5434

TomasLongo · 2025-02-13T08:08:39Z

Description

Updates the otel proto buf specifications.

This is a breaking change, since the otel proto buf specification removes InstrumentationLibrary* objects and replaces them with Scope* object. This also changes the hierarchy of the data structure

Issues Resolved

relates to #5322

Check List

New functionality includes testing.
New functionality has a documentation issue. Please link to it in this PR.
- New functionality has javadoc added
Commits are signed with a real name per the DCO

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.

Signed-off-by: Tomas Longo <tomas.longo@sap.com>

KarstenSchnitter

Thanks for submitting this PR. Let's try to get rid of the TODOs. I am not sure about the Kafka integration tests. The seem to fail in the build pipeline. Maybe @kkondaka can support?

...o-common/src/test/java/org/opensearch/dataprepper/plugins/otel/codec/OTelProtoCodecTest.java

.../src/main/java/org/opensearch/dataprepper/plugins/source/oteltrace/OTelTraceGrpcService.java

...c/integrationTest/java/org/opensearch/dataprepper/integration/trace/EndToEndRawSpanTest.java

Signed-off-by: Tomas Longo <tomas.longo@sap.com>

KarstenSchnitter

The huge number of deletions in the OTelMetricsProtoHelper look scary. But the methods were unused due to the migration to the OTelProtoCodec. There are still test failures in the Kafka integration tests. I triggered the build pipeline, so that we get a full picture. @kkondaka can you assist with the Kafka tests?

kkondaka · 2025-02-19T02:21:47Z

@TomasLongo looks like some tests are failing because of ScopeSpans related error in 0.9.0 version. Could you check?

@TomasLongo @KarstenSchnitter what's the impact of updating to latest version with customers who may be using older version to send data?

kkondaka · 2025-02-19T06:48:45Z

@TomasLongo I think we should remove 0.9 0-alpha from .github/workflows/data-prepper-trace-analytics-raw-span-e2e-tests.yml and use the new version you are adding there for testing. That way we can getrid of the error you are currently seeing in the failing e2e tests. @dlvenable what do you think?

KarstenSchnitter · 2025-02-20T09:11:50Z

In the PR introducing the instrumentationScope (open-telemetry/opentelemetry-proto#362) it clearly states:

This is not a breaking change for OTLP. The wire format does not change.

This is also evident from the *.proto files. With the upgrade to v1.3.2, the legacy check for instrumentationLibrary is removed. This affects senders, that use a protobuf definitions in the range v0.15.0 (March 19th, 2022) till v0.19.0 (August 3rd, 2022) still giving an instrumentationLibrary. Older implementations will be mapped to the instrumentationScope just fine. Later implementations no longer offer support for instrumentationLibrary.

I think we are good to go with the upgrade.

Signed-off-by: Tomas Longo <tomas.longo@sap.com>

TomasLongo · 2025-02-21T09:31:40Z

@kkondaka I've removed the old versions from the e2e-raw-span-test. At least, locally, the test is green now.

kkondaka · 2025-02-21T19:50:10Z

@KarstenSchnitter just to understand what you wrote, if some customer is still using OLD (pre feb-2021) logs generator which has "instrumentationLibrary", what happens when DataPrepper receives it, it will be dropped silently, right? (I understand there won't be any other issues or errors)

KarstenSchnitter · 2025-02-21T20:18:18Z

@kkondaka: If you used an Otel instrumentation pre-March 2022 it would generate a binary representation of the instrumentation library, that all post-March 2022 implementations would read as instrumentation scope. This is evident by the indices used in the *.proto files.

kkondaka · 2025-02-24T04:27:38Z

data-prepper-plugins/otel-proto-common/src/test/resources/test-request-multiple-logs.json

    "schemaUrl": "schemaurl"
-    },
-    {
-    "resource": {


Why "resource" section removed? I think it is still valid in the new protocol spec, right?

dlvenable · 2025-02-24T16:02:51Z

@kkondaka: If you used an Otel instrumentation pre-March 2022 it would generate a binary representation of the instrumentation library, that all post-March 2022 implementations would read as instrumentation scope. This is evident by the indices used in the *.proto files.

Thanks @KarstenSchnitter for elaborating on this. As I understand this is not a breaking change and I'm good to move forward.

Looking at the original PR I see that the index was the same, so this is backward compatible. The grace period was specifically to help the receiver (Data Prepper) and sender begin to migrate without immediate code changes. We've missed that, so we do have to change the code.

dlvenable · 2025-02-24T16:04:25Z

.github/workflows/data-prepper-trace-analytics-raw-span-e2e-tests.yml

@@ -14,7 +14,7 @@ jobs:
    strategy:
      matrix:
        java: [11, 17, 21, docker]
-        otelVersion: ['0.9.0-alpha', '0.16.0-alpha']
+        otelVersion: ['1.3.2-alpha']


Can we keep an old version in here and still have tests pass? I'd expect this to be ok since the wire format is unchanged.

otelVersion: ['0.16.0-alpha', '1.3.2-alpha']

@dlvenable I do not think that's possible.

Why not? The wire format is the same is it not?

dlvenable · 2025-02-24T16:06:18Z

...in/java/org/opensearch/dataprepper/plugins/processor/otelmetrics/OTelMetricsProtoHelper.java

@@ -40,14 +31,7 @@ public final class OTelMetricsProtoHelper {

    private static final Logger LOG = LoggerFactory.getLogger(OTelMetricsProtoHelper.class);
    private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
-    private static final String SERVICE_NAME = "service.name";


Thank you for cleaning up these unused methods and names!

dlvenable · 2025-02-24T16:12:54Z

...proto-common/src/main/java/org/opensearch/dataprepper/plugins/otel/codec/OTelProtoCodec.java

@@ -198,17 +193,6 @@ protected Collection<OpenTelemetryLog> parseResourceLogs(ResourceLogs rs, final
            final Map<String, Object> resourceAttributes = OTelProtoCodec.getResourceAttributes(rs.getResource());
            final String schemaUrl = rs.getSchemaUrl();

-            Stream<OpenTelemetryLog> mappedInstrumentationLibraryLogs = rs.getInstrumentationLibraryLogsList()


Why are these removed and not changed to iterate of getScopeLogs()?

scopeLogs is being processed in rs.getScopeLogsList().stream() below

We already supported scopeLogs, etc. Great! I see this now.

dlvenable · 2025-02-24T16:13:52Z

...proto-common/src/main/java/org/opensearch/dataprepper/plugins/otel/codec/OTelProtoCodec.java

@@ -239,22 +223,6 @@ protected Map<String, ResourceSpans> splitResourceSpansByTraceId(final ResourceS
                }
            }

-            if (resourceSpans.getInstrumentationLibrarySpansList().size() > 0) {


Similar comment as one above: Why is this removed entirely?

@dlvenable that API doesn't exist, so it is removed. The if (resourceSpans.getScopeSpansList().size() > 0) { processing scopeSpans.

dlvenable · 2025-02-24T16:14:08Z

...proto-common/src/main/java/org/opensearch/dataprepper/plugins/otel/codec/OTelProtoCodec.java

@@ -306,38 +270,6 @@ private Map<String, List<ScopeSpans>> splitScopeSpansByTraceId(final List<ScopeS
            return result;
        }

-        private List<Span> parseInstrumentationLibrarySpans(final List<InstrumentationLibrarySpans> instrumentationLibrarySpansList,


Same question: Why is this removed entirely?

This API removed because the underlying function ( instrumentationLibrarySpans.getInstrumentationLibrary()) is not valid anymore

dlvenable · 2025-02-24T16:14:32Z

Thank you @TomasLongo for working on this important update!

kkondaka · 2025-02-24T21:47:39Z

@TomasLongo The new spec adds "attributes" under "instrumentationScope", we should be able to do instrumentationScope.getAttributesList() and put them in the event too.

TomasLongo requested review from sb2k16, chenqi0805, engechas, san81, srikanthjg, graytaylor0, dinujoh, kkondaka, KarstenSchnitter, dlvenable and oeyh as code owners February 13, 2025 08:08

TomasLongo mentioned this pull request Feb 13, 2025

Introduce otlp/http support #5322

Draft

4 tasks

Update otel proto buf specification

a1c3d6c

Signed-off-by: Tomas Longo <tomas.longo@sap.com>

TomasLongo force-pushed the update-otel-proto-spec branch from 862fbe2 to a1c3d6c Compare February 13, 2025 08:19

KarstenSchnitter reviewed Feb 13, 2025

View reviewed changes

Remove todos

77feae9

Signed-off-by: Tomas Longo <tomas.longo@sap.com>

TomasLongo force-pushed the update-otel-proto-spec branch from 0749be9 to 77feae9 Compare February 13, 2025 13:32

remove last remnants of instrumentation library

103a7ae

Signed-off-by: Tomas Longo <tomas.longo@sap.com>

KarstenSchnitter reviewed Feb 14, 2025

View reviewed changes

Just test the new otel proto buf spec in the e2e raw span test

7c5b99d

Signed-off-by: Tomas Longo <tomas.longo@sap.com>

kkondaka reviewed Feb 24, 2025

View reviewed changes

dlvenable reviewed Feb 24, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update otel proto buf specification #5434

Update otel proto buf specification #5434

TomasLongo commented Feb 13, 2025

KarstenSchnitter left a comment

KarstenSchnitter left a comment

kkondaka commented Feb 19, 2025

kkondaka commented Feb 19, 2025

KarstenSchnitter commented Feb 20, 2025

TomasLongo commented Feb 21, 2025

kkondaka commented Feb 21, 2025

KarstenSchnitter commented Feb 21, 2025

kkondaka Feb 24, 2025

dlvenable commented Feb 24, 2025

dlvenable Feb 24, 2025

kkondaka Feb 24, 2025

dlvenable Feb 24, 2025

dlvenable Feb 24, 2025

dlvenable Feb 24, 2025

kkondaka Feb 24, 2025

dlvenable Feb 24, 2025

dlvenable Feb 24, 2025

kkondaka Feb 24, 2025

dlvenable Feb 24, 2025

kkondaka Feb 24, 2025

dlvenable commented Feb 24, 2025

kkondaka commented Feb 24, 2025

Update otel proto buf specification #5434

Are you sure you want to change the base?

Update otel proto buf specification #5434

Conversation

TomasLongo commented Feb 13, 2025

Description

Issues Resolved

Check List

KarstenSchnitter left a comment

Choose a reason for hiding this comment

KarstenSchnitter left a comment

Choose a reason for hiding this comment

kkondaka commented Feb 19, 2025

kkondaka commented Feb 19, 2025

KarstenSchnitter commented Feb 20, 2025

TomasLongo commented Feb 21, 2025

kkondaka commented Feb 21, 2025

KarstenSchnitter commented Feb 21, 2025

Choose a reason for hiding this comment

dlvenable commented Feb 24, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dlvenable commented Feb 24, 2025

kkondaka commented Feb 24, 2025