Update otel proto buf specification by TomasLongo · Pull Request #5434 · opensearch-project/data-prepper

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

      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?

Is this the OTEL version on the client side? If yes, that should work.

I have verified this locally and it should work.

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

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


    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

            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

                }
            }

-            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

            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.

KarstenSchnitter · 2025-02-25T19:28:19Z

The instrumentationScope attributes can be added with their own prefix similar to this example. This means to use instrumentationScope.attributes.namespace@dot@mapped. This is the approach taken so far for any attributes. We can use the same approach in #5259 for these attributes.

dlvenable · 2025-02-25T20:42:23Z

@KarstenSchnitter , @kkondaka ,

Would you be ok with this approach?

Approve and merge this PR.
Add the missing end-to-end test
Add the attributes

KarstenSchnitter · 2025-02-26T05:29:27Z

@dlvenable your proposal looks good to me.

* Update otel proto buf specification Signed-off-by: Tomas Longo <tomas.longo@sap.com> * Remove todos Signed-off-by: Tomas Longo <tomas.longo@sap.com> * remove last remnants of instrumentation library Signed-off-by: Tomas Longo <tomas.longo@sap.com> * Just test the new otel proto buf spec in the e2e raw span test Signed-off-by: Tomas Longo <tomas.longo@sap.com> --------- Signed-off-by: Tomas Longo <tomas.longo@sap.com> Co-authored-by: Tomas Longo <tomas.longo@sap.com>

dlvenable · 2025-03-04T20:44:07Z

@KarstenSchnitter , @kkondaka ,

Would you be ok with this approach?
1. Approve and merge this PR.

2. Add the missing end-to-end test

3. Add the attributes

The last two items (2 and 3) were completed in #5471.

* Update otel proto buf specification Signed-off-by: Tomas Longo <tomas.longo@sap.com> * Remove todos Signed-off-by: Tomas Longo <tomas.longo@sap.com> * remove last remnants of instrumentation library Signed-off-by: Tomas Longo <tomas.longo@sap.com> * Just test the new otel proto buf spec in the e2e raw span test Signed-off-by: Tomas Longo <tomas.longo@sap.com> --------- Signed-off-by: Tomas Longo <tomas.longo@sap.com> Co-authored-by: Tomas Longo <tomas.longo@sap.com> Signed-off-by: George Chen <qchea@amazon.com>

* Update otel proto buf specification Signed-off-by: Tomas Longo <tomas.longo@sap.com> * Remove todos Signed-off-by: Tomas Longo <tomas.longo@sap.com> * remove last remnants of instrumentation library Signed-off-by: Tomas Longo <tomas.longo@sap.com> * Just test the new otel proto buf spec in the e2e raw span test Signed-off-by: Tomas Longo <tomas.longo@sap.com> --------- Signed-off-by: Tomas Longo <tomas.longo@sap.com> Co-authored-by: Tomas Longo <tomas.longo@sap.com>

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

TomasLongo mentioned this pull request Feb 13, 2025

Introduce otlp/http support in OTelTraceSource #5322

Merged

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

dlvenable mentioned this pull request Feb 25, 2025

[Bug] Data-Prepper otel_logs_source: Incorrect OTLP Log Structure Mapping #5455

Open

dlvenable approved these changes Feb 26, 2025

View reviewed changes

kkondaka approved these changes Feb 26, 2025

View reviewed changes

kkondaka merged commit 20db890 into opensearch-project:main Feb 26, 2025
65 of 69 checks passed

dlvenable added this to the v2.11 milestone Mar 13, 2025

dinujoh mentioned this pull request Jun 10, 2025

[BUG] Exception when OTel metrics data is sent in JSON format with instrumentationLibrary field #5769

Open

Conversation

TomasLongo commented Feb 13, 2025

Description

Issues Resolved

Check List

Uh oh!

KarstenSchnitter left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

KarstenSchnitter left a comment

Choose a reason for hiding this comment

Uh oh!

kkondaka commented Feb 19, 2025

Uh oh!

kkondaka commented Feb 19, 2025

Uh oh!

KarstenSchnitter commented Feb 20, 2025

Uh oh!

TomasLongo commented Feb 21, 2025

Uh oh!

kkondaka commented Feb 21, 2025

Uh oh!

KarstenSchnitter commented Feb 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dlvenable commented Feb 24, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dlvenable commented Feb 24, 2025

Uh oh!

kkondaka commented Feb 24, 2025

Uh oh!

KarstenSchnitter commented Feb 25, 2025

Uh oh!

dlvenable commented Feb 25, 2025

Uh oh!

KarstenSchnitter commented Feb 26, 2025

Uh oh!

Uh oh!

dlvenable commented Mar 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants