Otel metrics source http service by TomasLongo · Pull Request #6604 · opensearch-project/data-prepper

TomasLongo · 2026-03-04T14:26:05Z

Description

Introduce HTTP/protobuf and HTTP/JSON support for OTel Metrics source.
Adds endpoint to receive OTLP data over HTTP. Aligns with similar support
for OTelTraceSource.

Issues Resolved

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.

TomasLongo · 2026-03-05T11:58:10Z

Note: I added the E2E Test for the metrics source to the log sub module in the e2e-test project.

It seemed a bit of an overkill to me to create a new module. Especially, since the already present tests for logs really just cover the happy path of the process.

I'd suggest to rename the log module to signals (or something similar) to encourage adding basic e2e tests that cover signals in that module.

More specialized/complex use cases can still be put into separate modules.

Integrates an HTTP (non-gRPC) service into the OTel metrics source plugin, mirroring the existing pattern from otel_trace_source and otel_logs_source. Both gRPC and HTTP services now run on the same Armeria server. Key changes: - Add ArmeriaHttpService for handling HTTP metric export requests - Add HttpExceptionHandler for HTTP-specific error handling - Support compression, authentication, throttling, and health checks for HTTP - Add configurable http_path option - Refactor OTelMetricsSource to directly configure the server - Remove ConvertConfiguration (inlined into source) - Split monolithic test into focused test classes (gRPC, HTTP, RetryInfo) - Add E2E tests for HTTP, gRPC, protobuf, and unframed requests Signed-off-by: Tomas Longo <tlongo@sternad.de> Signed-off-by: Kai Sternad <kai@sternad.de>

Signed-off-by: Kai Sternad <kai@sternad.de>

1. createServer() — configureHttpService() is now guarded with if (getHttpPath() != null), preventing the NPE 2. HTTP health check — moved from configureHttpService() into createServer(), so it registers when either httpPath or enableUnframedRequests is set (matching the OTelLogsSource pattern) 3. configureHttpService() — removed the duplicate health check registration Signed-off-by: Kai Sternad <kai@sternad.de>

Signed-off-by: Kai Sternad <kai@sternad.de>

KarstenSchnitter

Thanks for providing this PR. This already looks pretty good. I found some minor issues with that.

KarstenSchnitter · 2026-03-10T11:30:40Z

+    public static final int DEFAULT_REQUEST_TIMEOUT_MS = 10000;
+    public static final int DEFAULT_PORT = 21891;
+    public static final int DEFAULT_THREAD_COUNT = 200;
+    public static final int DEFAULT_MAX_CONNECTION_COUNT = 500;
+    public static final boolean DEFAULT_SSL = true;
+    public static final boolean DEFAULT_ENABLED_UNFRAMED_REQUESTS = false;
+    public static final boolean DEFAULT_HEALTH_CHECK = false;
+    public static final boolean DEFAULT_PROTO_REFLECTION_SERVICE = false;
+    public static final boolean DEFAULT_USE_ACM_CERT_FOR_SSL = false;
+    public static final int DEFAULT_ACM_CERT_ISSUE_TIME_OUT_MILLIS = 120000;


Why do these constants need to be public now? Can't they keep the scope they had before?

KarstenSchnitter · 2026-03-10T11:32:01Z

    @JsonProperty(RETRY_INFO)
    private RetryInfoConfig retryInfo;

+    @JsonProperty("http_path")


"http_path" should probably be extracted to a constant similar to all other property names.

KarstenSchnitter · 2026-03-10T11:53:14Z

+        ScheduledThreadPoolExecutor executor = configureServer(serverBuilder);
+
+        configureGrpcService(serverBuilder, buffer);
+        if (oTelMetricsSourceConfig.getHttpPath() != null) {


Do I understand correctly, that the gRPC endpoint is always created but the HTTP endpoint relies on the definition of a path? Or is there alway a default path making this check pass always? I am wondering on the different checks for logs, metrics and traces to configure the HTTP endpoint:

metrics: see above

logs: always

traces: if unframed requests is not true

Yes, we should unify http-service creation.

If I'm not mistaken, we haven't had a discussion for the trace-source to flesh out the exact mechanic how we want to treat the creation of the two services:

should both be mandatory?

should they be optional, in which case we would need to make sure that at least one service is configured?

[other approaches]

Any preferences?

Let's create an issue in this project to discuss this. The behaviour should be aligned. This will be necessary for adding the support to the OTelSource later on.

@KarstenSchnitter Regarding the inconsistency across the three sources: metrics and logs have been aligned with the latest commit. Both gate the HTTP endpoint on httpPath != null:

if (!oTelTraceSourceConfig.enableUnframedRequests()) HTTP is created by default, when unframed_requests is enabled, the gRPC service itself accepts plain HTTP/JSON requests, making a separate HTTP endpoint redundant.

The HTTP health check condition is also aligned now between metrics and logs: both use (enableUnframedRequests || httpPath != null) && healthCheck.

KarstenSchnitter · 2026-03-10T11:56:38Z

+    /**
+     * Returns the used thread pool. Can be used by other parts of the server configuration
+     */
+    private @NonNull ScheduledThreadPoolExecutor configureServer(ServerBuilder serverBuilder) {


This is strange naming considering the return value.

Yes, wasn't happy either.

I pulled out the creation of the task executor.

KarstenSchnitter · 2026-03-10T12:00:24Z

+                .exceptionHandler(createGrpcExceptionHandler(oTelMetricsSourceConfig));
+        final OTelMetricsGrpcService oTelmetricsGrpcService = new OTelMetricsGrpcService(
+                (int) (oTelMetricsSourceConfig.getRequestTimeoutInMillis() * 0.8),
+                oTelMetricsSourceConfig.getOutputFormat() == OTelOutputFormat.OPENSEARCH ? new OTelProtoOpensearchCodec.OTelProtoDecoder() : new OTelProtoStandardCodec.OTelProtoDecoder(),


Suggested change

oTelMetricsSourceConfig.getOutputFormat() == OTelOutputFormat.OPENSEARCH ? new OTelProtoOpensearchCodec.OTelProtoDecoder() : new OTelProtoStandardCodec.OTelProtoDecoder(),

createOtelProtoDecoder(),

@TomasLongo did you missed this?

No. I thought you were reviewing an outdated version.

Everything's good on my machine 🤷 . This is what I see locally and git tells me, that everything is up to date.

What do you see on your machine?

KarstenSchnitter · 2026-03-10T12:06:11Z

+    }
+
+    @Test
+    void start_with_Health_configured_includes_HealthCheck_service() throws IOException {


please use CamelCase naming adjust the other tests as well.

KarstenSchnitter · 2026-03-10T12:07:34Z

+
+    @ParameterizedTest
+    @MethodSource("getPathParams")
+    void httpRequest_writesToBuffer_returnsSuccessfulResponse(String givenPath, String resolvedRequestPath) throws Exception {


please use CamelCase naming. Please adjust the other tests as well.

Signed-off-by: Tomas Longo <tlongo@sternad.de>

KarstenSchnitter

Thanks for applying the latest changes. There are still two open comments, can you address those as well? Please also comment on the failing tests: Is this caused by this PR or by other factors?

KarstenSchnitter · 2026-03-12T08:42:49Z

+        ScheduledThreadPoolExecutor executor = configureServer(serverBuilder);
+
+        configureGrpcService(serverBuilder, buffer);
+        if (oTelMetricsSourceConfig.getHttpPath() != null) {


Let's create an issue in this project to discuss this. The behaviour should be aligned. This will be necessary for adding the support to the OTelSource later on.

KarstenSchnitter · 2026-03-12T08:45:00Z

+#### Update the version of DataPrepperVersion
+
+If you have just created a release branch, you should also create a PR on the `main` branch to bump the version in `DataPrepperVersion`.
+
+Steps:
+1. Modify the `DataPrepperVersion` class to update `CURRENT_VERSION` to the next version.
+2. Create a PR targeting `main`
+
+Note: This step can be automated through [#4877](https://github.com/opensearch-project/data-prepper/issues/4877).
+


This still looks like an accidental change. Are you sure this should be contained in this PR?

I'm confused. The file doesn't appear in the changeset.

I thought the accidental change was the removal of the section.

KarstenSchnitter · 2026-03-12T08:45:36Z

+                .exceptionHandler(createGrpcExceptionHandler(oTelMetricsSourceConfig));
+        final OTelMetricsGrpcService oTelmetricsGrpcService = new OTelMetricsGrpcService(
+                (int) (oTelMetricsSourceConfig.getRequestTimeoutInMillis() * 0.8),
+                oTelMetricsSourceConfig.getOutputFormat() == OTelOutputFormat.OPENSEARCH ? new OTelProtoOpensearchCodec.OTelProtoDecoder() : new OTelProtoStandardCodec.OTelProtoDecoder(),


@TomasLongo did you missed this?

Signed-off-by: Kai Sternad <kai@sternad.de>

KarstenSchnitter

This looks good to me. @dlvenable or @kkondaka do you have anything against this?

This reverts commit bb61dbe.

Signed-off-by: Taylor Gray <tylgry@amazon.com> This reverts commit bb61dbe.

This reverts commit bb61dbe. Signed-off-by: Taylor Gray <tylgry@amazon.com>

TomasLongo requested review from KarstenSchnitter, dinujoh, dlvenable, engechas, graytaylor0, kkondaka, oeyh, san81, sb2k16 and srikanthjg as code owners March 4, 2026 14:26

TomasLongo force-pushed the otel-metrics-source-http-service branch 3 times, most recently from b702577 to 7083df5 Compare March 5, 2026 11:37

TomasLongo force-pushed the otel-metrics-source-http-service branch from 7083df5 to 48d9981 Compare March 5, 2026 12:21

kaimst force-pushed the otel-metrics-source-http-service branch from bed11cb to 117d402 Compare March 6, 2026 09:41

kaimst force-pushed the otel-metrics-source-http-service branch from d7121b8 to 01df9df Compare March 6, 2026 12:39

kaimst added 3 commits March 6, 2026 13:55

Kepp constant handling consistent with other sources

c57c238

Signed-off-by: Kai Sternad <kai@sternad.de>

Move healthCheck back to configureHttpService

9c9b0d5

Signed-off-by: Kai Sternad <kai@sternad.de>

KarstenSchnitter reviewed Mar 10, 2026

View reviewed changes

Incorporate review findings

a818f7a

Signed-off-by: Tomas Longo <tlongo@sternad.de>

TomasLongo force-pushed the otel-metrics-source-http-service branch from bd48863 to a818f7a Compare March 11, 2026 09:15

KarstenSchnitter reviewed Mar 12, 2026

View reviewed changes

Deduplicate output format, decompose createServer

58c9ead

Signed-off-by: Kai Sternad <kai@sternad.de>

KarstenSchnitter approved these changes Mar 12, 2026

View reviewed changes

KarstenSchnitter mentioned this pull request Mar 13, 2026

Fix requestsTooLarge metric reporting when decompression buffer overflows on armeria in otel logs source #6633

Merged

4 tasks

KarstenSchnitter merged commit bb61dbe into opensearch-project:main Mar 17, 2026
70 of 72 checks passed

graytaylor0 added a commit to graytaylor0/data-prepper that referenced this pull request Mar 19, 2026

Revert "Otel metrics source http service (opensearch-project#6604)"

142e4ea

This reverts commit bb61dbe.

graytaylor0 mentioned this pull request Mar 19, 2026

Revert "Otel metrics source http service (#6604)" #6656

Merged

4 tasks

graytaylor0 added a commit to graytaylor0/data-prepper that referenced this pull request Mar 19, 2026

Revert "Otel metrics source http service (opensearch-project#6604)"

213e4fa

Signed-off-by: Taylor Gray <tylgry@amazon.com> This reverts commit bb61dbe.

graytaylor0 added a commit that referenced this pull request Mar 19, 2026

Revert "Otel metrics source http service (#6604)" (#6656)

cb55f53

This reverts commit bb61dbe. Signed-off-by: Taylor Gray <tylgry@amazon.com>

	oTelMetricsSourceConfig.getOutputFormat() == OTelOutputFormat.OPENSEARCH ? new OTelProtoOpensearchCodec.OTelProtoDecoder() : new OTelProtoStandardCodec.OTelProtoDecoder(),
	createOtelProtoDecoder(),

Conversation

TomasLongo commented Mar 4, 2026

Description

Issues Resolved

Check List

Uh oh!

TomasLongo commented Mar 5, 2026

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!

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!

Uh oh!

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!

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!

KarstenSchnitter left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants