-
Notifications
You must be signed in to change notification settings - Fork 19
Custom spans #141
Custom spans #141
Conversation
Codecov Report
@@ Coverage Diff @@
## master #141 +/- ##
=======================================
Coverage 96.33% 96.33%
=======================================
Files 31 31
Lines 928 928
Branches 144 144
=======================================
Hits 894 894
Misses 34 34 Continue to review full report at Codecov.
|
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.
Awesome! Could you also create a test case that has a custom span?
Requested changes are done. Ready to review. |
this.callSleepApi(); | ||
}); | ||
} | ||
|
||
callSleepApi() { | ||
const xhr = new XMLHttpRequest(); | ||
// Create a child span for the XHR. | ||
const callSleepApiCustomSpan = tracing.tracer.startChildSpan({ name: 'Call Sleep API' }); |
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.
What would you think about removing this custom span since there will already be one for the XHR?
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.
I would like to keep it. My thought is to show in the example that is possible to create custom spans even if those tasks already generate automatic spans. I will add some comments to clarify more this example.
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.
OK, I think it's fine to keep. Is there another call that does not have a custom span on it?
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.
Well, the call to prime numbers API does not have a custom span, as it also is an XHR, I decided to not create a custom span for that.
packages/opencensus-web-instrumentation-zone/test/test-interaction-tracker.ts
Outdated
Show resolved
Hide resolved
this.callSleepApi(); | ||
}); | ||
} | ||
|
||
callSleepApi() { | ||
const xhr = new XMLHttpRequest(); | ||
// Create a child span for the XHR. | ||
const callSleepApiCustomSpan = tracing.tracer.startChildSpan({ name: 'Call Sleep API' }); |
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.
OK, I think it's fine to keep. Is there another call that does not have a custom span on it?
Update the user interaction client example creating custom spans for the different tasks the click handler runs.
Also, update a little the documentation on how to do it. I know the documentation is not complete now, however, it will be completely updated later in the next weeks for productionization, for now just update a bit the documentation on this.