Otel logs source http service #6250

TomasLongo · 2025-11-07T13:53:50Z

Description

This PR adds an HTTP-Service to the OtelLogsSource, which runs alongside the gRPC-Service under a separate endpoint.

Both Services share the same underlying Armeria-Server, running under the same port.

Breakdown

The PR is kind of heavy. The OtelLogsSource had to be restructured resulting in the refactoring of the present tests.

A quick breakdown of the changes will certainly help with the review

One Server, two services

OtelLogsSource is now inhabitetd by two services running under different paths behind the same port.
The path of the services can be configured by the two fields path and http_path

path is the already present field for configuring path of the grpc service and keeps its purpose
http_path will set the path for the new http service

Unframed Requests

Since unframed requests served the purpose of sending plain http requests to the grpc server, this functionality has been dropped. However, for backwardscompatibilty of the config, it is still present in the config data structure.

CreateServer

The class CreateServer deals with creating both, http and gRPC servers. However, in its current form, the intended use is to create a server with a single service. Two quick attempts to rewrite the Class for the purpose if this PR yielded no results.

So the final version of the server creation loosely resembles the functionality found in CreateServer.

Refactoring of tests

Many tests were making sure that the former gRPC server, with unframed requests enabled, would handle http requests correctly. I have dropped this tests completely, replacing them with a separate test class that handles tests regarding the http functionality. I added a few improvements along the way like

Getting rid of unnecessary mocks
Introducing parameterized tests
Using unified test data and fixtures

Decisions made and additional considerations

Getting completely rid of unframed requests

enable_unframed_requests has no effect any more, since its purpose has been replaced with the new http service

Configuration

A new field, http_path has been added to specify the path of the http service. However, one could extend the config to

enable the client to specify only on of both service by omitting one of both paths. Currently, both services will be instantiated.

CreateServer

I think that the intention of CreateServer, to provide a unified mechanism to bootstrap armeria servers, is absolutely the right thing todo. However, in its current form, it only supports creating servers with a single service.

It's worth thinking about how to refactor CreateServer to streamline server creation for future plugins. The challenge I encountered was how to best model the fact, that different services might need different server capabilities/configurations.

Figuring this out, would have gone well beyond the scope of this PR.

Issues Resolved

Resolves #[Issue number to be closed when this PR is merged]

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 <[email protected]>

TomasLongo · 2025-11-07T13:54:08Z

@KarstenSchnitter

KarstenSchnitter

Just a start of a review. Can you check the tests for unused imports. This is failing the CI builds. I need more time for the review. Please expect more comments to come.

KarstenSchnitter · 2025-11-07T18:03:29Z

...rc/main/java/org/opensearch/dataprepper/plugins/source/otellogs/http/ArmeriaHttpService.java

+    // no path provided. Will be set by config.
+    @Post("")
+    @Consumes(value = "application/json")
+    public ExportTraceServiceResponse exportLog(ExportLogsServiceRequest request) {


Why is this returning ExportTraceServiceResponse? Shouldn't this return a ExportLogServiceResponse?

Right. I copied the Http-Service from the Trace-Source. This is the remnant from it. Fix is pushed.

Signed-off-by: Tomas Longo <[email protected]>

KarstenSchnitter

I already have some comments. I still need to review the tests.

KarstenSchnitter · 2025-11-15T08:06:35Z

...rc/main/java/org/opensearch/dataprepper/plugins/source/otellogs/http/ArmeriaHttpService.java

+        try {
+            logs = oTelProtoDecoder.parseExportLogsServiceRequest(request, Instant.now());
+        } catch (Exception e) {
+            LOG.error("Failed to parse the request {} due to:", request, e);


Please use warning level and sensitive request marker.

Suggested change

LOG.error("Failed to parse the request {} due to:", request, e);

LOG.warn(DataPrepperMarkers.SENSITIVE, "Failed to parse request with error '{}'. Request body: {}.", e.getMessage(), request);

done.

Is this a general rule? Use WARN-level with SENSITIVE-Marker for errors?

Is there any documentation on how to logging should be done in DataPrepper? Could not find any in the README

KarstenSchnitter · 2025-11-15T08:07:58Z

...rc/main/java/org/opensearch/dataprepper/plugins/source/otellogs/http/ArmeriaHttpService.java

+            throw new BadRequestException(e.getMessage(), e);
+        }
+
+        final List<Record<Object>> records = logs.stream().map(log -> new Record<Object>(log)).collect(Collectors.toList());


These records are only required in the else block of the following condition. Please move this operation to this block.

...-source/src/main/java/org/opensearch/dataprepper/plugins/source/otellogs/OTelLogsSource.java

KarstenSchnitter · 2025-11-15T08:32:34Z

...-source/src/main/java/org/opensearch/dataprepper/plugins/source/otellogs/OTelLogsSource.java

+
+        if (oTelLogsSourceConfig.hasHealthCheck()) {
+            LOG.info("HTTP source health check is enabled");
+            serverBuilder.service("/health", HealthCheckService.builder().longPolling(0).build());


Please extract the String to a constant.

KarstenSchnitter · 2025-11-15T08:42:18Z

...-source/src/main/java/org/opensearch/dataprepper/plugins/source/otellogs/OTelLogsSource.java

+            serverBuilder.maxRequestLength(oTelLogsSourceConfig.getMaxRequestLength().getBytes());
+        }
+        final int threadCount = oTelLogsSourceConfig.getThreadCount();
+        serverBuilder.blockingTaskExecutor(new ScheduledThreadPoolExecutor(threadCount), true);


Please use the same logic as in CreateServer to configure the task executor.

Signed-off-by: Tomas Longo <[email protected]>

KarstenSchnitter

This looks good so far. I would like to see an end-to-end test with OTLP/HTTP if possible. Can you add it? Please promote this PR to ready for review.

@TomasLongo please also fix the unused imports in the tests that lead to the failing Gradle builds.

TomasLongo · 2025-11-21T13:49:11Z

Yes. I'll add an E2E Test.

Signed-off-by: Tomas Longo <[email protected]>

TomasLongo added 10 commits November 3, 2025 12:42

[WIP]

62ce414

Signed-off-by: Tomas Longo <[email protected]>

[WIP]

84d4b54

Signed-off-by: Tomas Longo <[email protected]>

[WIP]

a02156f

Signed-off-by: Tomas Longo <[email protected]>

[WIP] Integrate http service and make sure it works properly

bc3edb8

Signed-off-by: Tomas Longo <[email protected]>

Integrate grpc and http service into a single server

916e02c

Signed-off-by: Tomas Longo <[email protected]>

Refactoring. Cleanup tests

1885a1d

Signed-off-by: Tomas Longo <[email protected]>

Extract tests that assert grpc requests

130bd5d

Signed-off-by: Tomas Longo <[email protected]>

Clean up

1b1b586

Signed-off-by: Tomas Longo <[email protected]>

Revert introduction of CertificatProvider interface

2d41fb2

Signed-off-by: Tomas Longo <[email protected]>

Cleanup

ec1980c

Signed-off-by: Tomas Longo <[email protected]>

KarstenSchnitter reviewed Nov 7, 2025

View reviewed changes

Fix return value of http service

20c396d

Signed-off-by: Tomas Longo <[email protected]>

TomasLongo force-pushed the otel-logs-source-http-service branch 2 times, most recently from 3cf2c0f to 1dd14dd Compare November 10, 2025 12:01

Re-introduce unframed request for the grpc service

965b93d

Signed-off-by: Tomas Longo <[email protected]>

TomasLongo force-pushed the otel-logs-source-http-service branch from 1dd14dd to 965b93d Compare November 14, 2025 07:18

KarstenSchnitter reviewed Nov 15, 2025

View reviewed changes

Incorporate review suggestions

656475c

Signed-off-by: Tomas Longo <[email protected]>

TomasLongo force-pushed the otel-logs-source-http-service branch from e05da93 to 656475c Compare November 21, 2025 08:27

KarstenSchnitter reviewed Nov 21, 2025

View reviewed changes

Add E2E test

5f2aeb3

Signed-off-by: Tomas Longo <[email protected]>

TomasLongo force-pushed the otel-logs-source-http-service branch from f6c5b40 to 5f2aeb3 Compare November 26, 2025 07:12

	LOG.error("Failed to parse the request {} due to:", request, e);
	LOG.warn(DataPrepperMarkers.SENSITIVE, "Failed to parse request with error '{}'. Request body: {}.", e.getMessage(), request);

Otel logs source http service #6250

Are you sure you want to change the base?

Otel logs source http service #6250

Uh oh!

Conversation

TomasLongo commented Nov 7, 2025

Description

Breakdown

One Server, two services

Unframed Requests

CreateServer

Refactoring of tests

Decisions made and additional considerations

Issues Resolved

Check List

Uh oh!

TomasLongo commented Nov 7, 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!

TomasLongo Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TomasLongo commented Nov 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

TomasLongo Nov 10, 2025 •

edited

Loading

KarstenSchnitter left a comment •

edited

Loading