Set g.co/agent in Stackdriver exported via Node info#604
Set g.co/agent in Stackdriver exported via Node info#604pjanotti merged 5 commits intocensus-instrumentation:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #604 +/- ##
=========================================
+ Coverage 68.97% 69.5% +0.52%
=========================================
Files 93 92 -1
Lines 6125 6142 +17
=========================================
+ Hits 4225 4269 +44
+ Misses 1679 1646 -33
- Partials 221 227 +6
Continue to review full report at Codecov.
|
|
@songy23 and @pjanotti would you be open to merging this PR even though the OpenCensus service is being replaced by the OpenTelemetry service? I noticed that the OpenTelemetry service doesn't have a Stackdriver exporter yet, so I think merging this PR would make it easier to adopt this feature in OpenTelemetry if we base the Stackdriver exporter on the OpenCensus service one. |
|
Yes, this should be done first here (OC) then we can pick it up on the contrib side of OpenTelemetry when that gets ready. |
pjanotti
left a comment
There was a problem hiding this comment.
Hi @draffensperger, thanks for the PR. A few comments and Qs.
| return "opencensus-" + getLanguageName(li.Language) + " " + li.CoreLibraryVersion + "; ocagent-exporter " + li.ExporterVersion | ||
| } | ||
|
|
||
| func getLanguageName(l commonpb.LibraryInfo_Language) string { |
There was a problem hiding this comment.
iirc proto can transform this in a string, any specific reason to have this function?
There was a problem hiding this comment.
Good point, I changed to calling .String() on the language enum. Initially I was hoping to make these agent labels match what is set from the libraries by default (e.g. opencensus-node rather than opencensus-node_js), but I think standardizing on the proto enum names is fine. I did lowercase it so that it's a little more similar to the way that agent labels get set on the per-language exporters.
|
It seems that code coverage numbers are wrong and don't reflect that actual PR numbers... |
|
Thanks for the reviews! I'll may not get to this for several days but I plan to fix it up per the comments. |
7d0d6d4 to
a355d13
Compare
…-service into write-agent-label
|
@pjanotti I think I've addressed all the comments (though wasn't sure how to easily remove the |
pjanotti
left a comment
There was a problem hiding this comment.
@draffensperger one last thing: could you double check the comment regarding the new interface?
…-service into write-agent-label
…-service into write-agent-label
|
@pjanotti friendly ping on this - are you comfortable with keeping the new interface instead of adding a dependency on |
|
hi @draffensperger - sorry, missed your reply earlier. I will merge this and cut a release later this week. |
|
@pjanotti awesome, thanks so much for your help with this! |
This uses the
Language,CoreLibraryVersionandExporterVersionfields of theLibraryInfomessage onNodeto set theg.co/agentspan label in the Stackdriver exporter.Because this code is running in the OpenCensus service (agent/collector) it's safe to assume that the exporter type in the application library code must have been the
ocagent-exporter. Hence, it sets theg.co/agentstring as follows:opencensus-[Language] [CoreLibraryVersion]; ocagent-exporter [ExporterVersion], e.g.opencensus-python 0.0.1; ocagent-exporter 0.0.2Fixes #525.