Instrument the SimpleLogProcessor from sdk/log
#7548
Conversation
yumosx
requested review from
MrAlias,
XSAM,
dashpole,
dmathieu,
flc1125 and
pellared
as code owners
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7548 +/- ##
=====================================
Coverage 86.1% 86.1%
=====================================
Files 297 297
Lines 21678 21693 +15
=====================================
+ Hits 18682 18695 +13
- Misses 2623 2624 +1
- Partials 373 374 +1
🚀 New features to boost your workflow:
|
|
|
||
| if s.inst != nil { | ||
| defer func() { | ||
| s.inst.LogProcessed(ctx, err) |
There was a problem hiding this comment.
I'm thinking that when this part of the logic is triggered, this section should also be reported.
if s.exporter == nil {
return nil
}There was a problem hiding this comment.
@flc1125, I don't quite understand this part.
could you explain it in detail?
There was a problem hiding this comment.
What I want to express is that we should also report this metric regarding this part of the situation.
However, what's a bit special here is that what is returned here is nil, but I'm wondering, when reporting the metric, should we define a custom error to describe the fact that it wasn't successfully exported (something like: simple processor has no exporter configured).
cc @MrAlias, if you have any good suggestions, please feel free to add them.
There was a problem hiding this comment.
@flc1125 about this issue, maybe we could address this in an separate PR, WDYT?
There was a problem hiding this comment.
I think we at least need to support indicators reporting for this scenario; the point of disagreement lies in the issue of err or nil.
if s.exporter == nil {
return nil
}Regarding the points of disagreement, I think it's fine to discuss them in another PR or issue.
Co-authored-by: Flc゛ <[email protected]>
# Conflicts: # CHANGELOG.md
2 participants
ref #7016
goarch: amd64 pkg: go.opentelemetry.io/otel/sdk/log cpu: Intel(R) Core(TM) i7-14700 │ before.txt │ .\after_res.txt │ │ sec/op │ sec/op vs base │ SimpleProcessorObservability/NoObservability-28 27.68n ± 4% SimpleProcessorObservability/Observability-28 27.39n ± 5% geomean 27.68n 27.38n ? ¹ ² ¹ benchmark set differs from baseline; geomeans may not be comparable ² ratios must be >0 to compute geomean │ before.txt │ .\after_res.txt │ │ B/op │ B/op vs base │ SimpleProcessorObservability/NoObservability-28 0.000 ± 0% SimpleProcessorObservability/Observability-28 0.000 ± 0% geomean ¹ ? ² ¹ ³ ¹ summaries must be >0 to compute geomean ² benchmark set differs from baseline; geomeans may not be comparable ³ ratios must be >0 to compute geomean │ before.txt │ .\after_res.txt │ │ allocs/op │ allocs/op vs base │ SimpleProcessorObservability/NoObservability-28 0.000 ± 0% SimpleProcessorObservability/Observability-28 0.000 ± 0% geomean ¹ ? ² ¹ ³ ¹ summaries must be >0 to compute geomean ² benchmark set differs from baseline; geomeans may not be comparable ³ ratios must be >0 to compute geomean