Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add instrumentation library name and version to OTLP exported metrics #1418

Merged
merged 3 commits into from
Nov 25, 2020
Merged

Add instrumentation library name and version to OTLP exported metrics #1418

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
33f7060
Add instrumentation library name and version to OTLP exported metrics
ocelotl Nov 25, 2020
287257f
Merge branch 'master' into issue_1417
lzchen Nov 25, 2020
25d36e1
Merge branch 'master' into issue_1417
Nov 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions exporter/opentelemetry-exporter-otlp/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## Unreleased

- Add instrumentation library name and version to OTLP exported metrics
([#1418](https://github.com/open-telemetry/opentelemetry-python/pull/1418))
- Change temporality for Counter and UpDownCounter
([#1384](https://github.com/open-telemetry/opentelemetry-python/pull/1384))
- Add Gzip compression for exporter
Expand Down
19 changes: 17 additions & 2 deletions .../opentelemetry-exporter-otlp/src/opentelemetry/exporter/otlp/metrics_exporter/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,9 +304,11 @@ def _translate_data(
)
argument = type_class[value_type]["gauge"]["argument"]

sdk_resource_instrumentation_library_metrics[
instrumentation_library_metrics = sdk_resource_instrumentation_library_metrics[
export_record.resource
].metrics.append(
]

instrumentation_library_metrics.metrics.append(
OTLPMetric(
**{
"name": export_record.instrument.name,
Expand All @@ -317,6 +319,19 @@ def _translate_data(
)
)

instrumentation_library_metrics.instrumentation_library.name = (
export_record.instrument.meter.instrumentation_info.name
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should the instrumentation info not be pulled from the resource on the export_record?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't look like Observers have a meter, filed a bug #1424

)

version = (
export_record.instrument.meter.instrumentation_info.version
)

if version:
(
instrumentation_library_metrics.instrumentation_library.version
) = version

return ExportMetricsServiceRequest(
resource_metrics=_get_resource_data(
sdk_resource_instrumentation_library_metrics,
Expand Down
1 change: 0 additions & 1 deletion ...er/opentelemetry-exporter-otlp/src/opentelemetry/exporter/otlp/trace_exporter/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
"""OTLP Span Exporter"""

import logging
import os
from typing import Optional, Sequence

from grpc import ChannelCredentials
Expand Down
8 changes: 6 additions & 2 deletions exporter/opentelemetry-exporter-otlp/tests/test_otlp_metric_exporter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
)
from opentelemetry.proto.common.v1.common_pb2 import (
AnyValue,
InstrumentationLibrary,
KeyValue,
StringKeyValue,
)
Expand Down Expand Up @@ -61,7 +62,7 @@ def setUp(self, mock_time_ns): # pylint: disable=arguments-differ
"d",
"e",
int,
MeterProvider(resource=resource,).get_meter(__name__),
MeterProvider(resource=resource,).get_meter("name", "version"),
("f",),
),
[("g", "h")],
Expand Down Expand Up @@ -121,6 +122,9 @@ def test_translate_metrics(self):
),
instrumentation_library_metrics=[
InstrumentationLibraryMetrics(
instrumentation_library=InstrumentationLibrary(
name="name", version="version",
),
metrics=[
OTLPMetric(
name="c",
Expand All @@ -145,7 +149,7 @@ def test_translate_metrics(self):
is_monotonic=True,
),
)
]
],
)
],
)
Expand Down