Skip to content

fix: Unify OTEL exporter diagnostics toggle and align headers#2571

Merged
abdulraqeeb33 merged 3 commits intomainfrom
fix/otel-fixes
Mar 12, 2026
Merged

fix: Unify OTEL exporter diagnostics toggle and align headers#2571
abdulraqeeb33 merged 3 commits intomainfrom
fix/otel-fixes

Conversation

@abdulraqeeb33
Copy link
Contributor

@abdulraqeeb33 abdulraqeeb33 commented Mar 12, 2026

Summary

  • centralize OTEL exporter diagnostic logging behind a single provider-level toggle (OTEL_EXPORTER_LOGGING_ENABLED surfaced as isOtelExporterLoggingEnabled) so all OTEL export paths use one source of truth
  • add exporter-level diagnostics that log OTEL export lifecycle (request sent, success, failure) and enrich failures with parsed HTTP details (status, status message, and response body when available)
  • keep OTEL transport compatible with current edge behavior by using the SDK header expected by the HTTP stack and explicitly validating that legacy blocked header variants are not sent
  • normalize OTEL endpoint construction to .../sdk/log?app_id={appId} and add coverage to prevent regressions in path/query formatting
  • remove redundant setAllAttributes(Attributes) extension in favor of the platform API/built-in method to reduce duplicated behavior and avoid extension shadowing noise
  • expand OTEL tests to cover header shape and endpoint formation, alongside existing exporter/provider wiring checks

Test plan

  • Run ./gradlew :OneSignal:otel:test
  • Run ./gradlew :OneSignal:otel:compileDebugKotlin
  • Verify branch diff against origin/main only includes OTEL/header and platform-provider toggle wiring changes

updated header name
@abdulraqeeb33 abdulraqeeb33 changed the title Unify OTEL exporter diagnostics toggle and align headers fix: Unify OTEL exporter diagnostics toggle and align headers Mar 12, 2026
@github-actions
Copy link
Contributor

📊 Diff Coverage Report

Diff Coverage Report (Changed Lines Only)

Threshold: 80%

Changed Files Coverage

  • OtelPlatformProvider.kt: 0/1 changed lines (0.0%) (5 changed lines)
    • ⚠️ Below threshold: 1 uncovered changed lines
  • OneSignalOpenTelemetry.kt: 0/8 changed lines (0.0%) (8 changed lines)
    • ⚠️ Below threshold: 8 uncovered changed lines
  • OtelConfigRemoteOneSignal.kt: 0/43 changed lines (0.0%) (85 changed lines)
    • ⚠️ Below threshold: 43 uncovered changed lines

Overall Coverage (Changed Lines Only)

0/52 changed lines covered (0.0%)

❌ Coverage Check Failed

Files below 80% threshold:

  • OtelPlatformProvider.kt: 0.0% (1 uncovered changed lines)

  • OneSignalOpenTelemetry.kt: 0.0% (8 uncovered changed lines)

  • OtelConfigRemoteOneSignal.kt: 0.0% (43 uncovered changed lines)

📥 View workflow run

}
}

override val isOtelExporterLoggingEnabled: Boolean = OTEL_EXPORTER_LOGGING_ENABLED
Copy link
Contributor

Choose a reason for hiding this comment

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

how does isOtelExporterLoggingEnabled get set to true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we set it manually whenever we want to debug in our local test env.

@nan-li nan-li self-requested a review March 12, 2026 18:29

val extraHttpHeaders: Map<String, String> by lazy {
mapOf(
"X-OneSignal-App-Id" to appId,
Copy link
Contributor

Choose a reason for hiding this comment

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

we don't need app_id in header anymore because it's already captured elsewhere right

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thats right

@abdulraqeeb33 abdulraqeeb33 merged commit ca99454 into main Mar 12, 2026
15 of 16 checks passed
@abdulraqeeb33 abdulraqeeb33 deleted the fix/otel-fixes branch March 12, 2026 18:50
abdulraqeeb33 added a commit that referenced this pull request Mar 12, 2026
Co-authored-by: AR Abdul Azeez <abdul@onesignal.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants