Add instrumentation library name and version to OTLP exported metrics#1418
Add instrumentation library name and version to OTLP exported metrics#1418codeboten merged 3 commits intoopen-telemetry:masterfrom
Conversation
NathanielRN
left a comment
There was a problem hiding this comment.
Nice! Love the metrics attention :)
codeboten
left a comment
There was a problem hiding this comment.
Just one question, then I can move to approving this PR and merging.
| ) | ||
|
|
||
| instrumentation_library_metrics.instrumentation_library.name = ( | ||
| export_record.instrument.meter.instrumentation_info.name |
There was a problem hiding this comment.
should the instrumentation info not be pulled from the resource on the export_record?
There was a problem hiding this comment.
Those values can possible be different I think? I'm wondering what "instrumentation library" in the context of metrics means. For [tracing[(https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/overview.md#instrumentation-libraries) it is instrumentation library (like flask), but here there is no such thing. Whereas Resource is defined as the entity that is producing the telemetry.
We should find out which is the most appropriate for what otlp needs in this field.
There was a problem hiding this comment.
I can't find the instrumentation info in the resource object:
ipdb> export_record.resource
<opentelemetry.sdk.resources.Resource object at 0x7f4c390dc730>
ipdb> dir(export_record.resource)
['__class__', '__delattr__', '__dict__', '__dir__', '__doc__', '__eq__', '__format__', '__ge__', '__getattribute__', '__gt__', '__hash__', '__init__', '__init_subclass__', '__le__', '__lt__', '__module__', '__ne__', '__new__', '__reduce__', '__reduce_ex__', '__repr__', '__setattr__', '__sizeof__', '__str__', '__subclasshook__', '__weakref__', '_attributes', 'attributes', 'create', 'create_empty', 'merge']
ipdb> export_record.resource
<opentelemetry.sdk.resources.Resource object at 0x7f4c390dc730>
ipdb> export_record.resource.attributes
OrderedDict([('a', 1), ('b', False)])
ipdb> export_record.resource.attributes
attributes create() create_empty() merge()
instance
This happens because we can have a Resource without those attributes, but the Meter will have them.
There was a problem hiding this comment.
I'm wondering what "instrumentation library" in the context of metrics means. For [tracing[(https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/overview.md#instrumentation-libraries) it is instrumentation library (like flask), but here there is no such thing.
I think it is supposed to be the same thing from reading this. I think this code is correct
There was a problem hiding this comment.
It doesn't look like Observers have a meter, filed a bug #1424
Fixes #1417
Description
Add missing instrumentation library attributes to the OTLP exported metrics
Fixes # (issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Checklist: