feat: Introduce GRPC and unified export configuration with DSL#1562
feat: Introduce GRPC and unified export configuration with DSL#1562namanONcode wants to merge 3 commits intoopen-telemetry:mainfrom
Conversation
…otocol selection.
There was a problem hiding this comment.
Pull request overview
This pull request introduces comprehensive support for gRPC export protocol alongside the existing HTTP protocol in the OpenTelemetry Android agent. It provides three configuration approaches: protocol-specific DSLs (httpExport, grpcExport) and a unified export DSL that supports both protocols through a protocol parameter.
Changes:
- Added
ExportProtocolenum to distinguish between HTTP and GRPC protocols - Introduced
GrpcExportConfigurationandExportConfigurationclasses for gRPC-specific and unified export configuration - Implemented
GrpcEndpointConnectivityto handle gRPC endpoint details without path suffixes - Refactored initialization logic to dynamically select and instantiate appropriate exporters based on configured protocol
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| ExportProtocol.kt | New enum defining HTTP and GRPC protocol options |
| GrpcEndpointConnectivity.kt | New internal class implementing EndpointConnectivity for gRPC endpoints |
| GrpcExportConfiguration.kt | New DSL configuration class for gRPC-specific export settings with signal overrides |
| ExportConfiguration.kt | New unified DSL configuration supporting both HTTP and GRPC protocols |
| OpenTelemetryConfiguration.kt | Added grpcExport() and export() DSL methods and internal config storage |
| OpenTelemetryRumInitializer.kt | Refactored to resolve export endpoints based on configuration priority and create protocol-appropriate exporters |
| ExportProtocolTest.kt | Unit tests for the ExportProtocol enum |
| GrpcEndpointConnectivityTest.kt | Unit tests for GrpcEndpointConnectivity implementation |
| GrpcExportConfigurationTest.kt | Comprehensive tests for gRPC export configuration DSL |
| ExportConfigurationTest.kt | Comprehensive tests for unified export configuration DSL |
| OpenTelemetryConfigurationExportTest.kt | Integration tests for all export configuration options |
| OpenTelemetryRumInitializerTest.kt | Tests verifying initialization with various export configurations |
Comments suppressed due to low confidence (1)
android-agent/src/main/kotlin/io/opentelemetry/android/agent/OpenTelemetryRumInitializer.kt:39
- Important documentation was removed from the
initializefunction. The function should retain its KDoc comment explaining its purpose, parameters, and any important behavioral notes (such as the context parameter requirements regarding application context vs. activity context). This documentation is important for API consumers to understand how to use this public API correctly.
@JvmStatic
fun initialize(
context: Context,
configuration: (OpenTelemetryConfiguration.() -> Unit) = {},
): OpenTelemetryRum {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| internal val sessionConfig = SessionConfiguration() | ||
| internal val instrumentations = InstrumentationConfiguration(rumConfig) | ||
| internal var resourceAction: ResourceBuilder.() -> Unit = {} | ||
|
|
There was a problem hiding this comment.
Documentation was removed for the httpExport function. This function should retain its KDoc comment to explain its purpose and usage, following the pattern established in the codebase (see the documentation in HttpExportConfiguration.kt for reference).
| /** | |
| * Configures exporting of telemetry over HTTP using [HttpExportConfiguration]. | |
| */ |
| fun grpcExport(action: GrpcExportConfiguration.() -> Unit) { | ||
| grpcExportConfig = GrpcExportConfiguration().apply(action) | ||
| } | ||
|
|
There was a problem hiding this comment.
Missing documentation for the export function. This new public API function should have KDoc documentation explaining its purpose, usage, and behavior. It should clarify that this is a unified export configuration DSL that supports both HTTP and gRPC protocols.
| /** | |
| * Configures export settings using a unified DSL that can target multiple protocols. | |
| * | |
| * This API is the preferred way to configure telemetry export and supports both HTTP | |
| * and gRPC transports through a single [ExportConfiguration] entry point. It coexists | |
| * with [httpExport] and [grpcExport], which provide protocol-specific configuration. | |
| * | |
| * The supplied [action] is applied to a new [ExportConfiguration] instance whose | |
| * resulting state is stored and used when building the underlying exporters. | |
| */ |
|
|
||
| fun spans(action: EndpointConfiguration.() -> Unit) { | ||
| spansConfig.action() | ||
| } | ||
|
|
||
| fun logs(action: EndpointConfiguration.() -> Unit) { | ||
| logsConfig.action() | ||
| } | ||
|
|
There was a problem hiding this comment.
Missing documentation for the spans, logs, and metrics functions. These public API functions should have KDoc documentation explaining their purpose, following the pattern established in HttpExportConfiguration (see HttpExportConfiguration.kt:60-79).
| fun spans(action: EndpointConfiguration.() -> Unit) { | |
| spansConfig.action() | |
| } | |
| fun logs(action: EndpointConfiguration.() -> Unit) { | |
| logsConfig.action() | |
| } | |
| /** | |
| * Configures export options specific to span data for this gRPC exporter. | |
| * | |
| * Values set in this configuration override the top-level [endpoint], | |
| * [headers], and [compression] for span exports. | |
| * | |
| * @param action configuration block applied to the spans [EndpointConfiguration]. | |
| */ | |
| fun spans(action: EndpointConfiguration.() -> Unit) { | |
| spansConfig.action() | |
| } | |
| /** | |
| * Configures export options specific to log data for this gRPC exporter. | |
| * | |
| * Values set in this configuration override the top-level [endpoint], | |
| * [headers], and [compression] for log exports. | |
| * | |
| * @param action configuration block applied to the logs [EndpointConfiguration]. | |
| */ | |
| fun logs(action: EndpointConfiguration.() -> Unit) { | |
| logsConfig.action() | |
| } | |
| /** | |
| * Configures export options specific to metric data for this gRPC exporter. | |
| * | |
| * Values set in this configuration override the top-level [endpoint], | |
| * [headers], and [compression] for metric exports. | |
| * | |
| * @param action configuration block applied to the metrics [EndpointConfiguration]. | |
| */ |
| fun grpcExport(action: GrpcExportConfiguration.() -> Unit) { | ||
| grpcExportConfig = GrpcExportConfiguration().apply(action) | ||
| } | ||
|
|
||
| fun export(action: ExportConfiguration.() -> Unit) { | ||
| unifiedExportConfig = ExportConfiguration().apply(action) | ||
| } |
There was a problem hiding this comment.
The current implementation allows multiple export configurations to coexist (httpExport, grpcExport, and export), but there's no clear documentation about the precedence order when multiple are set. Based on the resolveExportEndpoints logic (OpenTelemetryRumInitializer.kt:76-101), the precedence is: unified export > grpcExport > httpExport. Consider adding validation to warn users or prevent setting multiple export configurations, or at minimum, document this precedence clearly in the function documentation to avoid confusion.
|
|
||
| @Test | ||
| fun `HTTP ordinal is 0`() { | ||
| assertThat(ExportProtocol.HTTP.ordinal).isEqualTo(0) | ||
| } | ||
|
|
||
| @Test | ||
| fun `GRPC ordinal is 1`() { | ||
| assertThat(ExportProtocol.GRPC.ordinal).isEqualTo(1) | ||
| } |
There was a problem hiding this comment.
Tests for enum ordinals (lines 42-49) are testing implementation details of Kotlin enums that are guaranteed by the language. The ordinal values depend on the declaration order and testing them creates a brittle test that will break if the enum values are reordered. Consider removing these tests as they don't provide meaningful value and can make refactoring unnecessarily difficult.
| @Test | |
| fun `HTTP ordinal is 0`() { | |
| assertThat(ExportProtocol.HTTP.ordinal).isEqualTo(0) | |
| } | |
| @Test | |
| fun `GRPC ordinal is 1`() { | |
| assertThat(ExportProtocol.GRPC.ordinal).isEqualTo(1) | |
| } |
| class GrpcExportConfiguration internal constructor() { | ||
| var endpoint: String = "" | ||
|
|
||
| var headers: Map<String, String> = emptyMap() |
There was a problem hiding this comment.
The property name headers is inconsistent with the naming convention used in HttpExportConfiguration, which uses baseHeaders for the same purpose. For API consistency, consider renaming this property to baseHeaders to match the pattern established in HttpExportConfiguration (see HttpExportConfiguration.kt:24).
| class GrpcExportConfiguration internal constructor() { | ||
| var endpoint: String = "" | ||
|
|
||
| var headers: Map<String, String> = emptyMap() | ||
|
|
There was a problem hiding this comment.
Missing property documentation for endpoint, headers, and compression. These public API properties should have KDoc documentation explaining their purpose and behavior, following the pattern established in HttpExportConfiguration (see HttpExportConfiguration.kt:16-28).
| class GrpcExportConfiguration internal constructor() { | |
| var endpoint: String = "" | |
| var headers: Map<String, String> = emptyMap() | |
| class GrpcExportConfiguration internal constructor() { | |
| /** | |
| * The base gRPC endpoint to which telemetry will be exported. | |
| * | |
| * This value is used for all signals (spans, logs, metrics) unless a | |
| * signal-specific endpoint is configured via [spans], [logs], or [metrics]. | |
| */ | |
| var endpoint: String = "" | |
| /** | |
| * Headers that will be sent with every gRPC export request. | |
| * | |
| * These headers are combined with any headers configured on the per-signal | |
| * [EndpointConfiguration] instances used by [spans], [logs], and [metrics]. | |
| */ | |
| var headers: Map<String, String> = emptyMap() | |
| /** | |
| * The compression algorithm to use for gRPC export requests. | |
| * | |
| * This acts as the default compression for all signals and may be overridden | |
| * in the per-signal [EndpointConfiguration], if supported. | |
| */ |
|
|
||
| import io.opentelemetry.android.agent.connectivity.Compression | ||
| import io.opentelemetry.android.agent.connectivity.GrpcEndpointConnectivity | ||
|
|
There was a problem hiding this comment.
Missing class-level documentation for GrpcExportConfiguration. This public API class should have KDoc documentation explaining its purpose, usage, and relationship to other export configuration classes, following the pattern established in HttpExportConfiguration (see HttpExportConfiguration.kt:11-13).
| /** | |
| * Configuration for exporting telemetry over gRPC from the Android agent DSL. | |
| * | |
| * This mirrors [HttpExportConfiguration] but targets gRPC exporters and is used from | |
| * the `export { grpc { ... } }` section of the agent configuration DSL. | |
| * | |
| * Use [endpoint], [headers], and [compression] for common settings, and | |
| * [spans], [logs], and [metrics] to override per-signal endpoints, headers, | |
| * or compression as needed. | |
| */ |
| class ExportConfiguration internal constructor() { | ||
| var protocol: ExportProtocol = ExportProtocol.HTTP | ||
|
|
||
| var endpoint: String = "" | ||
|
|
||
| var headers: Map<String, String> = emptyMap() | ||
|
|
There was a problem hiding this comment.
Missing property documentation for protocol, endpoint, headers, and compression. These public API properties should have KDoc documentation explaining their purpose and behavior, especially the protocol property which is key to this unified configuration approach.
| class ExportConfiguration internal constructor() { | |
| var protocol: ExportProtocol = ExportProtocol.HTTP | |
| var endpoint: String = "" | |
| var headers: Map<String, String> = emptyMap() | |
| class ExportConfiguration internal constructor() { | |
| /** | |
| * Export protocol to use for all telemetry signals. | |
| * | |
| * This value controls whether HTTP or gRPC exporters are created in [spansEndpoint], | |
| * [logsEndpoint], and [metricsEndpoint]. Per-signal configuration currently inherits | |
| * this global protocol selection. | |
| */ | |
| var protocol: ExportProtocol = ExportProtocol.HTTP | |
| /** | |
| * Default endpoint URL used for all telemetry signals. | |
| * | |
| * Each signal (`spans`, `logs`, `metrics`) can define its own endpoint in | |
| * [EndpointConfiguration]. If a signal-specific endpoint is blank, this global | |
| * [endpoint] is used instead via [chooseEndpoint]. | |
| */ | |
| var endpoint: String = "" | |
| /** | |
| * Global HTTP headers applied to all telemetry exports. | |
| * | |
| * These headers are merged with signal-specific headers configured via | |
| * [EndpointConfiguration.headers]. When merging, entries from this global [headers] | |
| * map override signal-specific headers that use the same key. | |
| */ | |
| var headers: Map<String, String> = emptyMap() | |
| /** | |
| * Default compression algorithm for all telemetry signals. | |
| * | |
| * A signal can override this by setting a non-null compression in its | |
| * [EndpointConfiguration]. If no per-signal compression is provided, this global | |
| * [compression] value is used by [chooseCompression]. | |
| */ |
…corresponding configuration methods to OpenTelemetryConfiguration.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1562 +/- ##
==========================================
+ Coverage 64.45% 67.04% +2.58%
==========================================
Files 160 161 +1
Lines 3182 3280 +98
Branches 326 332 +6
==========================================
+ Hits 2051 2199 +148
+ Misses 1030 984 -46
+ Partials 101 97 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@namanONcode we expect that you are working on resolving the current comments from copilot before we undertake human reviews. Thanks. |
@breedx-splk @fractalwrench please review any suggestions ? |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
android-agent/src/main/kotlin/io/opentelemetry/android/agent/OpenTelemetryRumInitializer.kt:39
- The public
initializefunction’s KDoc was removed. This method is a primary entry point for the library and the docs contained important guidance about requiring anApplicationcontext (and the attachBaseContext caveat). Consider re-adding that documentation (even if the implementation remains unchanged) to avoid accidental misuse by callers.
@JvmStatic
fun initialize(
context: Context,
configuration: (OpenTelemetryConfiguration.() -> Unit) = {},
): OpenTelemetryRum {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| class OpenTelemetryConfiguration internal constructor( | ||
| internal val rumConfig: OtelRumConfig = OtelRumConfig(), | ||
| internal val diskBufferingConfig: DiskBufferingConfigurationSpec = DiskBufferingConfigurationSpec(rumConfig), | ||
| /** | ||
| * Configures the [Clock] used for capturing telemetry. | ||
| */ | ||
| var clock: Clock = OtelAndroidClock(), | ||
| ) { |
There was a problem hiding this comment.
The KDoc describing the purpose of the clock configuration option was removed. Since clock is a user-configurable input that affects timestamps and session behavior, restoring the brief documentation here would help API consumers understand when/why they might override it (e.g., for tests).
| * Each signal ([spans], [logs], [metrics]) can define its own endpoint. | ||
| * If a signal-specific endpoint is blank, this global endpoint is used instead. |
There was a problem hiding this comment.
ExportConfiguration.endpoint is documented as a generic “endpoint URL”, but its meaning differs by protocol (HTTP uses it as a base URL to which /v1/{signal} is appended; gRPC uses it as the final endpoint passed to the gRPC exporter). Updating the property KDoc to spell out this protocol-dependent behavior would prevent misconfiguration (e.g., users providing a full /v1/traces URL when protocol=HTTP and getting a doubled path).
| * Each signal ([spans], [logs], [metrics]) can define its own endpoint. | |
| * If a signal-specific endpoint is blank, this global endpoint is used instead. | |
| * How this value is interpreted depends on [protocol]: | |
| * - When [ExportProtocol.HTTP] is used, this is treated as the base URL to which the | |
| * signal-specific REST path (for example, `/v1/traces`, `/v1/logs`, `/v1/metrics`) is | |
| * appended by the HTTP exporters. In this case, do not include the `/v1/{signal}` suffix | |
| * in this value to avoid a doubled path. | |
| * - When [ExportProtocol.GRPC] is used, this is passed directly to the gRPC exporter as the | |
| * full endpoint (including any required path or port). | |
| * | |
| * Each signal ([spans], [logs], [metrics]) can define its own endpoint via | |
| * [EndpointConfiguration.url]. If a signal-specific endpoint is blank, this global endpoint | |
| * is used instead, following the same protocol-dependent behavior described above. |
|
@breedx-splk all review has been resolved please go through for human suggestions ? |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (1)
android-agent/src/main/kotlin/io/opentelemetry/android/agent/OpenTelemetryRumInitializer.kt:39
- The KDoc for
initialize(explaining required application context and configuration behavior) was removed. Since this is a public entry point, please keep/restore the API documentation so callers understand context requirements and the DSL parameter semantics.
object OpenTelemetryRumInitializer {
@JvmStatic
fun initialize(
context: Context,
configuration: (OpenTelemetryConfiguration.() -> Unit) = {},
): OpenTelemetryRum {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| HttpEndpointConnectivity.forMetrics( | ||
| chooseEndpoint(metricsConfig), | ||
| metricsConfig.headers + headers, | ||
| chooseCompression(metricsConfig.compression), | ||
| ) |
There was a problem hiding this comment.
In metricsEndpoint(), metricsConfig.headers + headers makes global headers win on duplicate keys, which is the opposite of the stated override semantics in the method KDoc. Consider swapping the operands (or updating docs/tests) so per-signal headers can override global ones.
| spansConfig.headers + headers, | ||
| chooseCompression(spansConfig.compression), | ||
| ) | ||
|
|
||
| internal fun logsEndpoint(): GrpcEndpointConnectivity = | ||
| GrpcEndpointConnectivity.create( | ||
| chooseEndpoint(logsConfig), | ||
| logsConfig.headers + headers, | ||
| chooseCompression(logsConfig.compression), | ||
| ) | ||
|
|
||
| internal fun metricsEndpoint(): GrpcEndpointConnectivity = | ||
| GrpcEndpointConnectivity.create( | ||
| chooseEndpoint(metricsConfig), | ||
| metricsConfig.headers + headers, |
There was a problem hiding this comment.
In spansEndpoint(), headers are merged as spansConfig.headers + headers, which means the top-level headers override per-signal headers on key conflicts (right-hand side wins for Kotlin map +). This conflicts with the KDoc stating per-signal values override; consider reversing the merge order or updating the KDoc to reflect actual behavior.
| spansConfig.headers + headers, | |
| chooseCompression(spansConfig.compression), | |
| ) | |
| internal fun logsEndpoint(): GrpcEndpointConnectivity = | |
| GrpcEndpointConnectivity.create( | |
| chooseEndpoint(logsConfig), | |
| logsConfig.headers + headers, | |
| chooseCompression(logsConfig.compression), | |
| ) | |
| internal fun metricsEndpoint(): GrpcEndpointConnectivity = | |
| GrpcEndpointConnectivity.create( | |
| chooseEndpoint(metricsConfig), | |
| metricsConfig.headers + headers, | |
| headers + spansConfig.headers, | |
| chooseCompression(spansConfig.compression), | |
| ) | |
| internal fun logsEndpoint(): GrpcEndpointConnectivity = | |
| GrpcEndpointConnectivity.create( | |
| chooseEndpoint(logsConfig), | |
| headers + logsConfig.headers, | |
| chooseCompression(logsConfig.compression), | |
| ) | |
| internal fun metricsEndpoint(): GrpcEndpointConnectivity = | |
| GrpcEndpointConnectivity.create( | |
| chooseEndpoint(metricsConfig), | |
| headers + metricsConfig.headers, |
| spansConfig.headers + headers, | ||
| chooseCompression(spansConfig.compression), | ||
| ) | ||
|
|
||
| internal fun logsEndpoint(): GrpcEndpointConnectivity = | ||
| GrpcEndpointConnectivity.create( | ||
| chooseEndpoint(logsConfig), | ||
| logsConfig.headers + headers, | ||
| chooseCompression(logsConfig.compression), | ||
| ) | ||
|
|
||
| internal fun metricsEndpoint(): GrpcEndpointConnectivity = | ||
| GrpcEndpointConnectivity.create( | ||
| chooseEndpoint(metricsConfig), | ||
| metricsConfig.headers + headers, |
There was a problem hiding this comment.
In logsEndpoint(), the current merge order logsConfig.headers + headers makes global headers take precedence over per-signal headers for duplicate keys. If per-signal overrides are intended, swap the merge order (or adjust documentation accordingly).
| spansConfig.headers + headers, | |
| chooseCompression(spansConfig.compression), | |
| ) | |
| internal fun logsEndpoint(): GrpcEndpointConnectivity = | |
| GrpcEndpointConnectivity.create( | |
| chooseEndpoint(logsConfig), | |
| logsConfig.headers + headers, | |
| chooseCompression(logsConfig.compression), | |
| ) | |
| internal fun metricsEndpoint(): GrpcEndpointConnectivity = | |
| GrpcEndpointConnectivity.create( | |
| chooseEndpoint(metricsConfig), | |
| metricsConfig.headers + headers, | |
| headers + spansConfig.headers, | |
| chooseCompression(spansConfig.compression), | |
| ) | |
| internal fun logsEndpoint(): GrpcEndpointConnectivity = | |
| GrpcEndpointConnectivity.create( | |
| chooseEndpoint(logsConfig), | |
| headers + logsConfig.headers, | |
| chooseCompression(logsConfig.compression), | |
| ) | |
| internal fun metricsEndpoint(): GrpcEndpointConnectivity = | |
| GrpcEndpointConnectivity.create( | |
| chooseEndpoint(metricsConfig), | |
| headers + metricsConfig.headers, |
| spansConfig.headers + headers, | ||
| chooseCompression(spansConfig.compression), | ||
| ) | ||
|
|
||
| internal fun logsEndpoint(): GrpcEndpointConnectivity = | ||
| GrpcEndpointConnectivity.create( | ||
| chooseEndpoint(logsConfig), | ||
| logsConfig.headers + headers, | ||
| chooseCompression(logsConfig.compression), | ||
| ) | ||
|
|
||
| internal fun metricsEndpoint(): GrpcEndpointConnectivity = | ||
| GrpcEndpointConnectivity.create( | ||
| chooseEndpoint(metricsConfig), | ||
| metricsConfig.headers + headers, |
There was a problem hiding this comment.
In metricsEndpoint(), metricsConfig.headers + headers causes global headers to override per-signal headers on key collision, which is inconsistent with the documented override behavior. Consider reversing the merge order or updating docs/tests to match precedence.
| spansConfig.headers + headers, | |
| chooseCompression(spansConfig.compression), | |
| ) | |
| internal fun logsEndpoint(): GrpcEndpointConnectivity = | |
| GrpcEndpointConnectivity.create( | |
| chooseEndpoint(logsConfig), | |
| logsConfig.headers + headers, | |
| chooseCompression(logsConfig.compression), | |
| ) | |
| internal fun metricsEndpoint(): GrpcEndpointConnectivity = | |
| GrpcEndpointConnectivity.create( | |
| chooseEndpoint(metricsConfig), | |
| metricsConfig.headers + headers, | |
| headers + spansConfig.headers, | |
| chooseCompression(spansConfig.compression), | |
| ) | |
| internal fun logsEndpoint(): GrpcEndpointConnectivity = | |
| GrpcEndpointConnectivity.create( | |
| chooseEndpoint(logsConfig), | |
| headers + logsConfig.headers, | |
| chooseCompression(logsConfig.compression), | |
| ) | |
| internal fun metricsEndpoint(): GrpcEndpointConnectivity = | |
| GrpcEndpointConnectivity.create( | |
| chooseEndpoint(metricsConfig), | |
| headers + metricsConfig.headers, |
| fun testHttpExportDefaultConfig() { | ||
| assertNotNull(config.exportConfig) | ||
| assertEquals("", config.exportConfig.baseUrl) |
There was a problem hiding this comment.
exportConfig is a non-null val on OpenTelemetryConfiguration, so assertNotNull(config.exportConfig) is redundant. Consider removing it to keep the test focused on meaningful behavior assertions.
| HttpEndpointConnectivity.forTraces( | ||
| chooseEndpoint(spansConfig), | ||
| spansConfig.headers + headers, | ||
| chooseCompression(spansConfig.compression), | ||
| ) |
There was a problem hiding this comment.
In spansEndpoint(), headers are merged as spansConfig.headers + headers. With Kotlin map +, keys from the right-hand map win, so global headers override per-signal headers on key conflicts. This contradicts the KDoc that says signal-specific values override top-level settings; consider reversing the merge order (global first, then signal) or updating the documentation/expectations accordingly.
| HttpEndpointConnectivity.forLogs( | ||
| chooseEndpoint(logsConfig), | ||
| logsConfig.headers + headers, | ||
| chooseCompression(logsConfig.compression), | ||
| ) |
There was a problem hiding this comment.
In logsEndpoint(), the merge order logsConfig.headers + headers causes global headers to override per-signal headers when keys collide. If per-signal configuration is intended to override, reverse the merge order or adjust the docs to match the implemented precedence.
|
@namanONcode there are 2 blockers to start reviewing this from my POV:
Once you resolve those two blockers then I'll find time to review contributions. |
|
@namanONcode we need you to circle back on this and address the things that @fractalwrench mentioned above, as well as resolving the large number of ai review comments and the merge conflict. Thanks! |
breedx-splk
added
the
needs author feedback
This pull request introduces support for configuring OpenTelemetry export protocols (HTTP and gRPC) in a unified and flexible way. It adds new configuration classes, updates the initialization logic to select the appropriate exporter based on protocol, and enhances test coverage to verify all configuration paths.
Export protocol support and configuration:
ExportProtocolenum to represent supported export protocols (HTTP, GRPC).ExportConfigurationfor unified protocol configuration andGrpcExportConfigurationfor gRPC-specific settings. [1] [2]GrpcEndpointConnectivityto encapsulate gRPC endpoint details.OpenTelemetryConfigurationto allow configuration viahttpExport,grpcExport, or unifiedexportDSLs, and store the selected configuration.Initialization logic enhancements:
OpenTelemetryRumInitializerto resolve export endpoints and protocol based on the selected configuration, and dynamically create the correct exporters for spans, logs, and metrics (supporting both HTTP and gRPC). [1] [2] [3] [4]Testing improvements:
ExportProtocolenum to ensure correct behavior.…otocol selection.