Skip to content

Fix breaking changes in otel_logs_source#6572

Merged
graytaylor0 merged 1 commit intoopensearch-project:mainfrom
oeyh:fix-otel-logs-source-http-regressions
Feb 27, 2026
Merged

Fix breaking changes in otel_logs_source#6572
graytaylor0 merged 1 commit intoopensearch-project:mainfrom
oeyh:fix-otel-logs-source-http-regressions

Conversation

@oeyh
Copy link
Copy Markdown
Collaborator

@oeyh oeyh commented Feb 27, 2026

Description

Commit 8d85609 (#6250) added HTTP service support to otel_logs_source but introduced regressions that break existing users:

  1. NPE on startup — configureHttpService() was called unconditionally, causing String.replace() to throw on a null http_path for any user who only configured a gRPC path.
  2. ACM certificate TLS silently broken — The SSL setup branch dropped the useAcmCertForSSL() check, so users relying on ACM-issued certificates would start without TLS.
  3. HTTP health check registered unconditionally — The old behavior gated the HTTP /health endpoint on enableUnframedRequests() being true; the new code registered it for all configs regardless.
  4. Auth provider split — The server-level HTTP decorator was loaded as an ArmeriaHttpAuthenticationProvider while the gRPC interceptor used a separate GrpcAuthenticationProvider, potentially
    resolving to different plugin implementations. The original code used a single GrpcAuthenticationProvider for both.

All changes are confined to OTelLogsSource.java:

  • Gate configureHttpService() on http_path being non-null — making HTTP service opt-in, consistent with how gRPC path works
  • Restore || useAcmCertForSSL() in the SSL condition
  • Restore the HTTP health check gate: registered only when enableUnframedRequests() or http_path is set (the latter ensures the new HTTP service also gets a health endpoint)
  • Hoist GrpcAuthenticationProvider creation into createServer, apply its getHttpAuthenticationService() as the server-level decorator, and pass the same instance into configureGrpcService —
    removing the split-provider pattern and the now-dead createHttpAuthentication() helper

Issues Resolved

N/A

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.

…vice support

Signed-off-by: Hai Yan <oeyh@amazon.com>
@Test
void start_withoutHttpPath_doesNotThrowNPE() {
final OTelLogsSourceConfig config = createDefaultConfigBuilder()
.httpPath(null)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should invert this. Most tests should set httpPath to null and we only have one that sets it.

I'm fine with merging this fix now, but think we should update in a quick follow-on.

@dlvenable dlvenable modified the milestones: v2.15, v2.14.1 Feb 27, 2026
@graytaylor0 graytaylor0 merged commit 1bc5459 into opensearch-project:main Feb 27, 2026
73 of 74 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Feb 27, 2026
…vice support (#6572)

Signed-off-by: Hai Yan <oeyh@amazon.com>
(cherry picked from commit 1bc5459)
oeyh added a commit that referenced this pull request Feb 27, 2026
…vice support (#6572) (#6574)

(cherry picked from commit 1bc5459)

Signed-off-by: Hai Yan <oeyh@amazon.com>
Co-authored-by: Hai Yan <8153134+oeyh@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants