feat: Instrument @platformatic/kafka #3486
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3486 +/- ##
==========================================
- Coverage 97.75% 97.62% -0.14%
==========================================
Files 414 427 +13
Lines 55577 56363 +786
Branches 1 1
==========================================
+ Hits 54330 55025 +695
- Misses 1247 1338 +91
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| * @private | ||
| * @interface | ||
| */ | ||
| class Subscriber { |
There was a problem hiding this comment.
I'm assuming this interface class is meant to be the true parent of all three subscribers: Subscriber (base.js), DiagnosticsChannelSubscriber (dc-base.js) and now TracingChannelSubscriber?
There was a problem hiding this comment.
Possibly. The intention is to figure out the commonality once I figure out how all of the details here.
| * Update the current context's timing tracker. | ||
| */ | ||
| touch() { | ||
| const context = this.agent.tracer.getContext() |
There was a problem hiding this comment.
this isn't the segment you created above. it's already into the net library. I'm unclear why you created an entirely new subscriber. All that logic to bind the segment to tracing channel event, etc lives in our base subscriber. The only thing that had to be edited there was to change the id it subscribes to events. But this is your issue why you see Truncated names because the segment that's being created isn't getting touched. Doing this fixes your name though:
diff --git a/lib/subscribers/platformatic-kafka/index.js b/lib/subscribers/platformatic-kafka/index.js
index 851018df9..ec630fc9a 100644
--- a/lib/subscribers/platformatic-kafka/index.js
+++ b/lib/subscribers/platformatic-kafka/index.js
@@ -54,6 +54,7 @@ const connectsSub = new TracingChannelSubscription({
const producerSendsSub = new TracingChannelSubscription({
channel: 'tracing:plt:kafka:producer:sends',
start(event) {
+ debugger
const context = this.agent.tracer.getContext()
const { segment, transaction } = context
@@ -88,20 +89,23 @@ const producerSendsSub = new TracingChannelSubscription({
externalSegment.type = 'message'
externalSegment.start()
+ event.segment = externalSegment
return context.enterSegment({ segment: externalSegment })
},
- end() {
- this.touch()
+ end(data) {
+ this.touch(data)
},
- asyncStart() {
+ asyncStart(data) {
+ debugger
// TODO: are we getting the correct context? Do we need to do the crazy
// `isActiveTx` context propagation as we had to do in other cases?
- this.touch()
+ this.touch(data)
},
- asyncEnd() {
- this.touch()
+ asyncEnd(data) {
+ debugger
+ this.touch(data)
}
})
@@ -117,9 +121,13 @@ class PlatformaticKafkaSubscriber extends TracingChannelSubscriber {
/**
* Update the current context's timing tracker.
*/
- touch() {
- const context = this.agent.tracer.getContext()
- context?.segment?.touch()
+ touch(data) {
+ if (data.segment) {
+ data?.segment?.touch()
+ } else {
+ const context = this.agent.tracer.getContext()
+ context?.segment?.touch()
+ }
}
}
There was a problem hiding this comment.
I'm unclear why you created an entirely new subscriber.
Because the existing code is all either tied to Orchestrion based channels or to baseline diagnostics channel. This module exports baseline tracing channel stuff. So we need something that handles that.
There was a problem hiding this comment.
Right but the only thing tied to orchestrion is
node-newrelic/lib/subscribers/base.js
Line 78 in 63e531b
|
|
||
| const connectsSub = new TracingChannelSubscription({ | ||
| channel: 'tracing:plt:kafka:connections:connects', | ||
| start(event) { |
There was a problem hiding this comment.
This segment is getting named as Truncated/connect as well because segment is never touched
3 participants
WIP