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

Resource is missing in MetricRecord for MetricExporter #1022

Closed
legendecas opened this issue May 6, 2020 · 3 comments · Fixed by #1049
Closed

Resource is missing in MetricRecord for MetricExporter #1022

legendecas opened this issue May 6, 2020 · 3 comments · Fixed by #1049
Assignees
Labels
enhancement New feature or request feature-request

Comments

@legendecas
Copy link
Member

legendecas commented May 6, 2020

Is your feature request related to a problem? Please describe.
MetricExporter can not get access to Metric's resource in the current API.

Describe the solution you'd like
Add a parameter for resources like go's implementation https://github.com/open-telemetry/opentelemetry-go/blob/master/exporters/metric/prometheus/prometheus.go#L171

MetricExporter.export(
  resource: Resource,
  metrics: MetricRecord[],
  resultCallback: (result: ExportResult) => void
): void;

However, altering the order of the parameters will be a breaking change.

@dyladan dyladan added the enhancement New feature or request label May 6, 2020
@dyladan
Copy link
Member

dyladan commented May 6, 2020

I am fine with this sort of breaking change as it is entirely contained within the SDK. We need to pass resource so I don't see a strong reason not to adopt this, we will just need to bump the minor version.

@dyladan
Copy link
Member

dyladan commented May 6, 2020

It looks like there is an open spec issue for this open-telemetry/opentelemetry-specification#508

@mwear
Copy link
Member

mwear commented May 6, 2020

I think the spec issue and the proposal above both make sense. We should discuss those further at the Spec SIG. For now, we should go with what is specified in the SDK Resource spec

It says:

Analogous to distributed tracing, when used with metrics, a resource can be associated with a MeterProvider. When associated with a MeterProvider, all Metrics produced by any Meter from the provider will be associated with this Resource.

Metric instances have a reference to a Resource. It just needs to be exposed on MetricRecord. See: https://github.com/open-telemetry/opentelemetry-js/blob/master/packages/opentelemetry-metrics/src/Metric.ts#L45.

You can assign this to me @dyladan.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feature-request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants