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

Jump to bottom

MetricProducer.Produce can include a Resource parameter #3636

Merged
merged 5 commits into from
Aug 10, 2023
Merged

MetricProducer.Produce can include a Resource parameter #3636

merged 5 commits into from
Aug 10, 2023

Conversation

dashpole
Copy link
Contributor

@dashpole dashpole commented Aug 1, 2023 •
edited
Loading

Fixes #3615

Changes

If a batch of metric points includes a resource, the MetricProducer needs to have a way to get resource in order to populate it. Rather than require metric producers to accept resource information on creation, add resource as a parameter to produce.

@dashpole dashpole marked this pull request as ready for review August 1, 2023 18:32
@dashpole dashpole requested review from a team August 1, 2023 18:32
@dashpole
Copy link
Contributor Author

dashpole commented Aug 1, 2023

@reyang @jsuereth @jack-berg @jmacd @aabmass @MrAlias

aabmass
aabmass approved these changes Aug 1, 2023
View reviewed changes
specification/metrics/sdk.md Outdated Show resolved Hide resolved
@aabmass aabmass mentioned this pull request Aug 1, 2023
Merged
4 tasks
dashpole
dashpole commented Aug 1, 2023
View reviewed changes
specification/metrics/sdk.md Outdated Show resolved Hide resolved
@dashpole dashpole changed the title Collect ignores resource information from metric producers MetricProducer.Produce can include a Resource parameter Aug 3, 2023
@dashpole dashpole mentioned this pull request Aug 3, 2023
Closed
jack-berg
jack-berg approved these changes Aug 4, 2023
View reviewed changes
Copy link
Member

@jack-berg jack-berg left a comment

Choose a reason for hiding this comment

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

This approach seems a bit more elegant to me, but as mentioned here, I don't think this materially changes anything since the spec already described how MeterProviders should configure MetricProducers with a Resource.

@reyang reyang merged commit fce0a69 into open-telemetry:main Aug 10, 2023
6 checks passed
@dashpole dashpole deleted the loosen_resource branch August 10, 2023 18:43
@aabmass aabmass mentioned this pull request Aug 23, 2023
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jsuereth jsuereth jsuereth approved these changes

@aabmass aabmass aabmass approved these changes

@jmacd jmacd jmacd approved these changes

@MrAlias MrAlias MrAlias approved these changes

@reyang reyang reyang approved these changes

@jack-berg jack-berg jack-berg approved these changes

Assignees

@yurishkuro yurishkuro

Labels
None yet
Projects
No open projects
Go: Metric SDK (GA)
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Loosen "MetricProducer MUST accept configuration for the Resource" requirement
8 participants
@dashpole @jsuereth @aabmass @jmacd @MrAlias @reyang @jack-berg @yurishkuro