[chore] Create service/internal/obsconsumer package #12817
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #12817 +/- ##
==========================================
+ Coverage 91.43% 91.46% +0.03%
==========================================
Files 488 493 +5
Lines 26879 26984 +105
==========================================
+ Hits 24577 24682 +105
Misses 1818 1818
Partials 484 484 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
djaglowski
force-pushed
the
obsconsumer
branch
2 times, most recently
from
94b6fcb to
9d93162
Compare
djaglowski
force-pushed
the
obsconsumer
branch
2 times, most recently
from
3a2603b to
712a269
Compare
| // Measure before calling ConsumeLogs because the data may be mutated downstream | ||
| itemCount := ld.LogRecordCount() | ||
|
|
||
| err := c.consumer.ConsumeLogs(ctx, ld) |
There was a problem hiding this comment.
would we also want to record a metric for after the consume for the amount produced?
There was a problem hiding this comment.
The way this is intended to be used is that there will be two instances of the wrapper, one measuring consumed and the other measuring produced. Measuring both in one instance would effectively mean that we need to enhance the model for edges so that each edge is aware of the attributes of the producing and consuming components. I think it ends up being a lot simpler this way.
There was a problem hiding this comment.
Should we document how this is intended to be used then? Or write a parent component that does the wrapping automatically? i.e. something that creates both instances wrapping the actual component Consume call?
There was a problem hiding this comment.
It's an internal package so I'm not too worried about documentation. #12812 uses it as I've described. The reason I've split it into a separate PR is to make it easier to test and review. I'm not sure further abstractions are helpful.
There was a problem hiding this comment.
I think this looks really good 👍🏻
I would also like to see bytes being recorded, but understand that's not part of this PR and would need to involve some serialisation like the ProtoMarshaler which has additional cost.
There was a problem hiding this comment.
LGTM, with two notes for posterity:
- As already mentioned in a comment thread, we'll need to implement a "downstream" error marker and the "rejected" outcome eventually
- I don't think I implemented component attribute injection for profiles, so there is probably additional work to be done on that front as well
Subset of #12812
This internal package defines wrappers around consumers. These are useful for instrumenting the component graph, so that we can generate telemetry describing data as it is passed in between components.
Currently, this supports only a single counter metric, but in the near future it can be enhanced to automatically capture multiple metrics (e.g. item count & size), and potentially spans and/or logs as well.