Add support for generating Standard(OTEL) conformant events in DataPrepper by kkondaka · Pull Request #5524 · opensearch-project/data-prepper

kkondaka · 2025-03-17T20:22:42Z

Description

Add support for generating Standard(OTEL) conformant events in DataPrepper.
Added option to otel-logs-source, otel-metrics-source and otel-trace-source to generate Standard/OTEL conformant events using output_format: otel option

Issues Resolved

Resolves #5259

Check List

[ X] New functionality includes testing.
New functionality has a documentation issue. Please link to it in this PR.
- New functionality has javadoc added
[X ] 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.

…epper Signed-off-by: Krishna Kondaka <krishkdk@amazon.com>

kkondaka · 2025-03-17T20:27:02Z

@dlvenable @KarstenSchnitter Please review.

ps48 · 2025-03-17T21:26:43Z

Hi @kkondaka, I do not know the internals of Data-prepper, just trying to understand few things on this change. Will This new output_format: otel still work with otel_traces_raw processor and when ingesting this to opensearch is the index_type still trace-analytics-raw ?
Thanks!

dlvenable · 2025-03-18T15:32:25Z

...common/src/main/java/org/opensearch/dataprepper/plugins/otel/codec/OTelLogsFormatOption.java


 public enum OTelLogsFormatOption {
    JSON("json"),
+    STANDARD_JSON("standard_json"),


I understand that the current format option is instructing the input format into the codec. That is, how do we decode.

But, in the difference between "standard" and "opensearch" we are directing the output from the codec.

I tend to think we should make this a new option instead of overloading this.

dlvenable · 2025-03-18T15:38:03Z

data-prepper-api/src/main/java/org/opensearch/dataprepper/model/metric/JacksonMetric.java


    protected static final String NAME_KEY = "name";
-    protected static final String SCOPE_KEY = "scope";
+    protected static final String SCOPE_KEY = "instrumentationScope";


This affects both standard and OpenSearch. Is this what we want?

It is not used in Opensearch mode. It already has other keys defined like this

static final String INSTRUMENTATION_SCOPE_NAME = "instrumentationScope.name"; static final String INSTRUMENTATION_SCOPE_VERSION = "instrumentationScope.version"; static final String INSTRUMENTATION_SCOPE_ATTRIBUTES = "instrumentationScope.attributes";

The new SCOPE_KEY with instrumentationScope is only used in the standard mode.

dlvenable · 2025-03-18T15:39:26Z

...on/src/main/java/org/opensearch/dataprepper/plugins/otel/codec/OTelLogsInputCodecConfig.java

+    @JsonProperty(value = "otel_format", defaultValue = "opensearch")
+    @JsonPropertyDescription("Specifies the format of the OTel Output.")
+    @NotNull
+    private OTelOutputFormat otelFormat = OTelOutputFormat.OPENSEARCH;


This makes sense.

But, why do you change the enum above? See my other comment.

As I clarified above. That was not needed. Removed it.

Signed-off-by: Krishna Kondaka <krishkdk@amazon.com>

kkondaka · 2025-03-18T16:25:41Z

@ps48 I have not verified the otel_traces_raw behavior with the new Standard mode. Will do that in my next PR.

KarstenSchnitter

I only did a partial review, but I wanted to give some timely feedback. I searched for some of the issues I commented on specifically. There might be more just around them.

KarstenSchnitter · 2025-03-19T20:36:59Z

...common/src/main/java/org/opensearch/dataprepper/plugins/otel/codec/OTelProtoCommonUtils.java

+        return Instant.ofEpochSecond(0L, unixNano).toString();
+    }
+
+    public static long timeISO8601ToNanos(final String timeISO08601) {


Just for symmetry with the other names:

Suggested change

public static long timeISO8601ToNanos(final String timeISO08601) {

public static long convertISO8601ToNanos(final String timeISO08601) {

Will address in my next PR.

KarstenSchnitter · 2025-03-19T20:44:22Z

...on/src/main/java/org/opensearch/dataprepper/plugins/otel/codec/OTelProtoOpensearchCodec.java

+                    .flatMap(rs -> parseResourceLogs(rs, timeReceived).stream()).collect(Collectors.toList());
+        }
+
+        protected Collection<OpenTelemetryLog> parseResourceLogs(ResourceLogs rs, final Instant timeReceived) {


I do not know, if this is the right time for changing the method signature. But how about returning a Stream>OpenTelemetryLog> here? This would avoid the creation of a collection at the end of the method just to stream it again at the caller.

Will investigate and address in the next PR.

KarstenSchnitter · 2025-03-19T20:48:47Z

...on/src/main/java/org/opensearch/dataprepper/plugins/otel/codec/OTelProtoOpensearchCodec.java

+        }
+        if (!instrumentationScope.getAttributesList().isEmpty()) {
+            for (Map.Entry<String, Object> entry: OTelProtoOpensearchCodec.unpackKeyValueList(instrumentationScope.getAttributesList()).entrySet()) {
+                instrumentationScopeAttr.put(INSTRUMENTATION_SCOPE_ATTRIBUTES+entry.getKey(), entry.getValue());


Isn't there a DOT missing in this concatenation similar to all the other attributes. For resource, logs, metrics, and span attributes there is a function being called to generate the attribute name.

Not required. The "unpackKeyValueList" is returning the attributes with "." as prefix!

KarstenSchnitter · 2025-03-19T20:49:41Z

...on/src/main/java/org/opensearch/dataprepper/plugins/otel/codec/OTelProtoOpensearchCodec.java

+                    .map(sls ->
+                            processLogsList(sls.getLogRecordsList(),
+                                    serviceName,
+                                    OTelProtoOpensearchCodec.getInstrumentationScopeAttributes(sls.getScope()),


Isn't this method located in this very class? Why is there an explicit class reference?

Will address in my next PR.

KarstenSchnitter · 2025-03-19T21:06:46Z

...mmon/src/main/java/org/opensearch/dataprepper/plugins/otel/codec/OTelProtoStandardCodec.java

+                        .withStartTime(OTelProtoStandardCodec.getStartTimeISO8601(dp))
+                        .withTime(OTelProtoStandardCodec.getTimeISO8601(dp))
+                        .withServiceName(serviceName)
+                        .withValue(OTelProtoStandardCodec.getValueAsDouble(dp))


I am not sure, that the standard gauge/sum/histogram data point should always use a double value. Maybe there should be a distinction between value.int and value.double as documented here: https://github.com/opensearch-project/opensearch-catalog/blob/main/docs/schema/observability/metrics/metrics.md

Will investigate and address in next PR. If you know, definitive answer, please let me know.

kkondaka · 2025-03-19T22:43:32Z

@KarstenSchnitter Please complete your review and let me know any other comments you may have. I have another PR coming up and I will address all your comments in that.

…epper (opensearch-project#5524) * Add support for generating Standard(OTEL) conformant events in DataPrepper Signed-off-by: Krishna Kondaka <krishkdk@amazon.com> * Addressed review comments Signed-off-by: Krishna Kondaka <krishkdk@amazon.com> --------- Signed-off-by: Krishna Kondaka <krishkdk@amazon.com>

…epper (opensearch-project#5524) * Add support for generating Standard(OTEL) conformant events in DataPrepper Signed-off-by: Krishna Kondaka <krishkdk@amazon.com> * Addressed review comments Signed-off-by: Krishna Kondaka <krishkdk@amazon.com> --------- Signed-off-by: Krishna Kondaka <krishkdk@amazon.com> Signed-off-by: George Chen <qchea@amazon.com>

…epper (opensearch-project#5524) * Add support for generating Standard(OTEL) conformant events in DataPrepper Signed-off-by: Krishna Kondaka <krishkdk@amazon.com> * Addressed review comments Signed-off-by: Krishna Kondaka <krishkdk@amazon.com> --------- Signed-off-by: Krishna Kondaka <krishkdk@amazon.com>

…epper (opensearch-project#5524) * Add support for generating Standard(OTEL) conformant events in DataPrepper Signed-off-by: Krishna Kondaka <krishkdk@amazon.com> * Addressed review comments Signed-off-by: Krishna Kondaka <krishkdk@amazon.com> --------- Signed-off-by: Krishna Kondaka <krishkdk@amazon.com> # Conflicts: # data-prepper-plugins/otel-logs-source/src/main/java/org/opensearch/dataprepper/plugins/source/otellogs/OTelLogsSource.java # data-prepper-plugins/otel-logs-source/src/main/java/org/opensearch/dataprepper/plugins/source/otellogs/OTelLogsSourceConfig.java # data-prepper-plugins/otel-metrics-source/src/main/java/org/opensearch/dataprepper/plugins/source/otelmetrics/OTelMetricsSource.java # data-prepper-plugins/otel-trace-source/src/main/java/org/opensearch/dataprepper/plugins/source/oteltrace/OTelTraceSource.java # data-prepper-plugins/otel-trace-source/src/main/java/org/opensearch/dataprepper/plugins/source/oteltrace/OTelTraceSourceConfig.java

…epper (opensearch-project#5524) * Add support for generating Standard(OTEL) conformant events in DataPrepper Signed-off-by: Krishna Kondaka <krishkdk@amazon.com> * Addressed review comments Signed-off-by: Krishna Kondaka <krishkdk@amazon.com> --------- Signed-off-by: Krishna Kondaka <krishkdk@amazon.com> Signed-off-by: mamol27 <mamol27@yandex.ru>

Add support for generating Standard(OTEL) conformant events in DataPr…

6fea535

…epper Signed-off-by: Krishna Kondaka <krishkdk@amazon.com>

kkondaka requested review from KarstenSchnitter, chenqi0805, dinujoh, dlvenable, engechas, graytaylor0, oeyh, san81, sb2k16 and srikanthjg as code owners March 17, 2025 20:22

dlvenable requested changes Mar 18, 2025

View reviewed changes

Addressed review comments

f62f20f

Signed-off-by: Krishna Kondaka <krishkdk@amazon.com>

dlvenable approved these changes Mar 19, 2025

View reviewed changes

KarstenSchnitter reviewed Mar 19, 2025

View reviewed changes

sb2k16 approved these changes Mar 19, 2025

View reviewed changes

kkondaka merged commit f136f48 into opensearch-project:main Mar 20, 2025
46 of 50 checks passed

kkondaka mentioned this pull request Mar 21, 2025

Support trace groups in standard otel proto codec #5539

Merged

2 tasks

rpasche mentioned this pull request Jun 18, 2025

Inconsistencies regarding ss4o in docs, code, index-templates #5791

Open

	public static long timeISO8601ToNanos(final String timeISO08601) {
	public static long convertISO8601ToNanos(final String timeISO08601) {

Conversation

kkondaka commented Mar 17, 2025

Description

Issues Resolved

Check List

Uh oh!

kkondaka commented Mar 17, 2025

Uh oh!

ps48 commented Mar 17, 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!

kkondaka commented Mar 18, 2025

Uh oh!

KarstenSchnitter left a comment

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!

kkondaka commented Mar 19, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants