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.

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

Support Resource in the Prometheus Exporter #867

Closed
mayurkale22 opened this issue Mar 17, 2020 · 12 comments · Fixed by #3300
Closed

Support Resource in the Prometheus Exporter #867

mayurkale22 opened this issue Mar 17, 2020 · 12 comments · Fixed by #3300
Assignees

Comments

@mayurkale22
Copy link
Member

The Resources were added to the default SDK in opentelemetry-js#846. They need to be integrated in the export workflow of the Prometheus exporter similar to opentelemetry-js#843

@mayurkale22 mayurkale22 added feature-request up-for-grabs Good for taking. Extra help will be provided by maintainers labels Mar 17, 2020
@mayurkale22 mayurkale22 added this to the After Beta Release milestone Mar 18, 2020
@mayurkale22 mayurkale22 self-assigned this Apr 29, 2020
@mayurkale22 mayurkale22 removed the up-for-grabs Good for taking. Extra help will be provided by maintainers label Apr 29, 2020
@michaelfig
Copy link

michaelfig commented Feb 15, 2022

What is the desired behaviour to support Resource? If there is clear guidance, I should be able to implement it.

I tried changing the PrometheusSerializer to append record.resource.attributes to the attributes for each metric unconditionally, but that looks like it might be too many attributes or too much data (especially for the unit tests, which have a full path to Node.js as the service_name).

@legendecas
Copy link
Member

It is going to be specified at open-telemetry/opentelemetry-specification#2266 to drop all resource attributes, only converting job and instance to Prometheus labels.

@dyladan dyladan added the up-for-grabs Good for taking. Extra help will be provided by maintainers label May 11, 2022
@dyladan dyladan modified the milestones: After Beta Release, Metrics GA Jun 1, 2022
@klacabane
Copy link

Hi! To clarify things on my end: is this about exposing the resource attributes passed, for example to MeterProvider, in the exporter as the "target" info family metric like described here ?

@dyladan
Copy link
Member

dyladan commented Jul 11, 2022

This is about including resource attributes as part of the serialized prometheus output data. As far as I know this behavior is actually not in the specification so this issue might actually just be closed as not planned. It is open so we can determine what other SIGs do with resource data in prometheus and attempt to be consistent.

@legendecas
Copy link
Member

We have spec texts on how to transform OpenTelemetry resource attributes to Prometheus labels: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/data-model.md#resource-attributes-1.

@klacabane
Copy link

klacabane commented Jul 12, 2022

Similar issue in the net open-telemetry/opentelemetry-dotnet#3087 and java sdk open-telemetry/opentelemetry-java#4552 with a follow up discussion on potential specification improvements open-telemetry/opentelemetry-specification#2640.

So we should be able to update the PrometheusSerializer.serialize function so that it also transforms the resource attributes available in the ResourceMetrics input to a target info group. Is there some other places that needs to be updated ?

@legendecas
Copy link
Member

So we should be able to update the PrometheusSerializer.serialize function so that it also transforms the resource attributes available in the ResourceMetrics input to a target info group.

Yes, that's where we need to update. Do you plan to work on this? If so I can assign this to you.

@dyladan
Copy link
Member

dyladan commented Jul 12, 2022

There is a related spec issue right now to determine precedence of keys open-telemetry/opentelemetry-specification#2535

@klacabane
Copy link

@legendecas I'm trying to evaluate the work required for our use case atm. I'm still questioning what should happen downstream, in the collectors, as the info type does not appear to be supported. I've tried two parsing libraries and both ended in a failure to parse the metric type info. So I assume this needs a corresponding implementation in the other end ?

@legendecas
Copy link
Member

@klacabane I'm not sure what you are referring to as the info type. I believe PrometheusSerializer.serialize is the place that needs to be updated to export resource attributes for downstream to take that into account.

@klacabane
Copy link

We need to transform the resource attributes into an info type in the PrometheusSerializer.serialize and I was wondering how the client pulling this data should interpret them. It's out of this issue's scope but I didn't find relevant informations. Currently the clients choke on the info metric type so my assumption is that clients would have to support this new type and have specific logic tying the attributes of the info block to the other metrics in the payload.

@dyladan dyladan modified the milestones: Metrics GA, Metrics After GA Aug 31, 2022
@pichlermarc
Copy link
Member

I can work on this 🙂

@pichlermarc pichlermarc removed the up-for-grabs Good for taking. Extra help will be provided by maintainers label Sep 28, 2022
@pichlermarc pichlermarc self-assigned this Sep 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants