-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: logs SDK observability - otlploggrpc exporter metrics #7353
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7353 +/- ##
=======================================
+ Coverage 85.5% 85.7% +0.1%
=======================================
Files 279 284 +5
Lines 24711 25001 +290
=======================================
+ Hits 21143 21436 +293
+ Misses 3189 3186 -3
Partials 379 379
🚀 New features to boost your workflow:
|
exporters/otlp/otlplog/otlploggrpc/internal/observ/instrumentation.go
Outdated
Show resolved
Hide resolved
exporters/otlp/otlplog/otlploggrpc/internal/observ/instrumentation.go
Outdated
Show resolved
Hide resolved
exporters/otlp/otlplog/otlploggrpc/internal/observ/instrumentation.go
Outdated
Show resolved
Hide resolved
exporters/otlp/otlplog/otlploggrpc/internal/observ/instrumentation_test.go
Outdated
Show resolved
Hide resolved
exporters/otlp/otlplog/otlploggrpc/internal/observ/instrumentation.go
Outdated
Show resolved
Hide resolved
exporters/otlp/otlplog/otlploggrpc/internal/observ/instrumentation.go
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the contribution. This is a significant step towards resolution, but there are few outstanding issues that need to be addressed:
- The scope name needs to be fixed
- Tests need to be updated to evaluate the scope and any other telemetry that is missing
- There are no benchmarks
exporters/otlp/otlplog/otlploggrpc/internal/observ/instrumentation.go
Outdated
Show resolved
Hide resolved
exporters/otlp/otlplog/otlploggrpc/internal/observ/instrumentation.go
Outdated
Show resolved
Hide resolved
exporters/otlp/otlplog/otlploggrpc/internal/observ/instrumentation.go
Outdated
Show resolved
Hide resolved
exporters/otlp/otlplog/otlploggrpc/internal/observ/instrumentation_test.go
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from some remaining issues, I have no problems with other aspects on my end.
Thank you for your hard work and contributions.
9f46a5a to
6bb0d6b
Compare
In order to add gRPC server attributes for exporter observability, this information needs to be parsed into a host and port. The added generated files provides ParseCanonicalTarget for this functionality. This is added as a generated template as it is expected to be needed for all OTLP exporters (e.g. open-telemetry#7404, open-telemetry#7353).
In order to add [gRPC server attributes for exporter observability](https://github.com/open-telemetry/semantic-conventions/blob/main/docs/otel/sdk-metrics.md#metric-otelsdkexporterspaninflight), this information needs to be parsed into a host and port. The added generated files provides `ParseCanonicalTarget` for this functionality. This is added as a generated template as it is expected to be needed for all OTLP exporters (e.g. #7404, #7353). Split from work added to #7404 cc @yumosx ### Benchamarks ``` goos: linux goarch: amd64 pkg: go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal/observ cpu: Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz │ otlptracegrpc-observ-target.bmark.result │ │ sec/op │ ParseTarget/HostName-8 80.90n ± 1% ParseTarget/HostPort-8 123.2n ± 4% ParseTarget/IPv4WithoutPort-8 94.25n ± 2% ParseTarget/IPv4WithPort-8 136.2n ± 1% ParseTarget/IPv6Bare-8 195.5n ± 2% ParseTarget/IPv6Bracket-8 191.2n ± 3% ParseTarget/IPv6WithPort-8 128.6n ± 4% ParseTarget/UnixSocket-8 15.73n ± 4% ParseTarget/UnixAbstractSocket-8 15.71n ± 6% ParseTarget/Passthrough-8 129.3n ± 18% geomean 84.98n │ otlptracegrpc-observ-target.bmark.result │ │ B/op │ ParseTarget/HostName-8 48.00 ± 0% ParseTarget/HostPort-8 48.00 ± 0% ParseTarget/IPv4WithoutPort-8 16.00 ± 0% ParseTarget/IPv4WithPort-8 48.00 ± 0% ParseTarget/IPv6Bare-8 16.00 ± 0% ParseTarget/IPv6Bracket-8 16.00 ± 0% ParseTarget/IPv6WithPort-8 48.00 ± 0% ParseTarget/UnixSocket-8 0.000 ± 0% ParseTarget/UnixAbstractSocket-8 0.000 ± 0% ParseTarget/Passthrough-8 48.00 ± 0% geomean ¹ ¹ summaries must be >0 to compute geomean │ otlptracegrpc-observ-target.bmark.result │ │ allocs/op │ ParseTarget/HostName-8 1.000 ± 0% ParseTarget/HostPort-8 1.000 ± 0% ParseTarget/IPv4WithoutPort-8 1.000 ± 0% ParseTarget/IPv4WithPort-8 1.000 ± 0% ParseTarget/IPv6Bare-8 1.000 ± 0% ParseTarget/IPv6Bracket-8 1.000 ± 0% ParseTarget/IPv6WithPort-8 1.000 ± 0% ParseTarget/UnixSocket-8 0.000 ± 0% ParseTarget/UnixAbstractSocket-8 0.000 ± 0% ParseTarget/Passthrough-8 1.000 ± 0% geomean ¹ ¹ summaries must be >0 to compute geomean ```
exporters/otlp/otlplog/otlploggrpc/internal/observ/instrumentation.go
Outdated
Show resolved
Hide resolved
exporters/otlp/otlplog/otlploggrpc/internal/observ/instrumentation.go
Outdated
Show resolved
Hide resolved
exporters/otlp/otlplog/otlploggrpc/internal/observ/instrumentation.go
Outdated
Show resolved
Hide resolved
exporters/otlp/otlplog/otlploggrpc/internal/observ/instrumentation.go
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good outside of the un-export of the added function.
This PR adds support for experimental metrics in
otlploggrpcotel.sdk.exporter.log.inflightotel.sdk.exporter.log.exportedotel.sdk.exporter.operation.durationReferences:
goos: darwin goarch: arm64 pkg: go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc cpu: Apple M3 │ disabled.txt │ enabled.txt │ │ sec/op │ sec/op vs base │ ExporterExportLogs/Observability-8 681.5µ ± 3% 684.3µ ± 6% ~ (p=0.315 n=10) │ disabled.txt │ enabled.txt │ │ B/op │ B/op vs base │ ExporterExportLogs/Observability-8 672.8Ki ± 0% 673.6Ki ± 1% ~ (p=0.247 n=10) │ disabled.txt │ enabled.txt │ │ allocs/op │ allocs/op vs base │ ExporterExportLogs/Observability-8 9.224k ± 0% 9.232k ± 0% +0.09% (p=0.000 n=10)goos: darwin goarch: arm64 pkg: go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc/internal/observ cpu: Apple M3 │ bench.txt │ │ sec/op │ InstrumentationExportLogs/NoError-8 162.6n ± 3% InstrumentationExportLogs/PartialError-8 705.5n ± 5% InstrumentationExportLogs/FullError-8 592.1n ± 1% geomean 408.0n │ bench.txt │ │ B/op │ InstrumentationExportLogs/NoError-8 152.0 ± 0% InstrumentationExportLogs/PartialError-8 697.0 ± 0% InstrumentationExportLogs/FullError-8 616.0 ± 0% geomean 402.6 │ bench.txt │ │ allocs/op │ InstrumentationExportLogs/NoError-8 3.000 ± 0% InstrumentationExportLogs/PartialError-8 10.00 ± 0% InstrumentationExportLogs/FullError-8 8.000 ± 0% geomean 6.214pkg: go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc/internal/observ cpu: Apple M3 │ parse_target.txt │ │ sec/op │ ParseTarget/HostName-8 38.00n ± ∞ ¹ ParseTarget/HostPort-8 51.33n ± ∞ ¹ ParseTarget/IPv4WithoutPort-8 44.74n ± ∞ ¹ ParseTarget/IPv4WithPort-8 62.56n ± ∞ ¹ ParseTarget/IPv6Bare-8 94.89n ± ∞ ¹ ParseTarget/IPv6Bracket-8 93.78n ± ∞ ¹ ParseTarget/IPv6WithPort-8 57.57n ± ∞ ¹ ParseTarget/UnixSocket-8 8.329n ± ∞ ¹ ParseTarget/UnixAbstractSocket-8 9.082n ± ∞ ¹ ParseTarget/Passthrough-8 58.06n ± ∞ ¹ geomean 40.64n ¹ need >= 6 samples for confidence interval at level 0.95 │ parse_target.txt │ │ B/op │ ParseTarget/HostName-8 48.00 ± ∞ ¹ ParseTarget/HostPort-8 48.00 ± ∞ ¹ ParseTarget/IPv4WithoutPort-8 16.00 ± ∞ ¹ ParseTarget/IPv4WithPort-8 48.00 ± ∞ ¹ ParseTarget/IPv6Bare-8 16.00 ± ∞ ¹ ParseTarget/IPv6Bracket-8 16.00 ± ∞ ¹ ParseTarget/IPv6WithPort-8 48.00 ± ∞ ¹ ParseTarget/UnixSocket-8 0.000 ± ∞ ¹ ParseTarget/UnixAbstractSocket-8 0.000 ± ∞ ¹ ParseTarget/Passthrough-8 48.00 ± ∞ ¹ geomean ² ¹ need >= 6 samples for confidence interval at level 0.95 ² summaries must be >0 to compute geomean │ parse_target.txt │ │ allocs/op │ ParseTarget/HostName-8 1.000 ± ∞ ¹ ParseTarget/HostPort-8 1.000 ± ∞ ¹ ParseTarget/IPv4WithoutPort-8 1.000 ± ∞ ¹ ParseTarget/IPv4WithPort-8 1.000 ± ∞ ¹ ParseTarget/IPv6Bare-8 1.000 ± ∞ ¹ ParseTarget/IPv6Bracket-8 1.000 ± ∞ ¹ ParseTarget/IPv6WithPort-8 1.000 ± ∞ ¹ ParseTarget/UnixSocket-8 0.000 ± ∞ ¹ ParseTarget/UnixAbstractSocket-8 0.000 ± ∞ ¹ ParseTarget/Passthrough-8 1.000 ± ∞ ¹ geomean ² ¹ need >= 6 samples for confidence interval at level 0.95 ² summaries must be >0 to compute geomean