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

[prometheusremotewriteexporter] Translate resource to the target info metric #8493

Merged
merged 7 commits into from
Apr 6, 2022
Merged

[prometheusremotewriteexporter] Translate resource to the target info metric #8493

merged 7 commits into from
Apr 6, 2022

Conversation

dashpole
Copy link
Contributor

@dashpole dashpole commented Mar 16, 2022 •
edited
Loading

Description:

Fixes #8265.

The only part that wasn't specified in the Spec is what to use as the timestamp of the target info metric. I've chosen to use the most recent timestamp in the batch.

Alternatives:

  • We could set it to time.Now(), but then it would have a different timestamp from the rest of the batch, which might cause some strange behavior.
  • We could also pick a random timestamp from the batch and use that. That would work well in the standard case in which all scraped prometheus metrics have their timestamp set to the scrape time.

Testing:

Added unit tests

@dashpole dashpole force-pushed the target_info_exporter branch 6 times, most recently from 98d09ab to 4b8dc3f Compare March 17, 2022 21:13
@dashpole dashpole marked this pull request as ready for review March 17, 2022 21:15
@dashpole dashpole requested a review from a team March 17, 2022 21:15
@dashpole dashpole requested a review from Aneurysm9 as a code owner March 17, 2022 21:15
@dashpole dashpole force-pushed the target_info_exporter branch from 4b8dc3f to 6c48675 Compare March 18, 2022 22:20
Aneurysm9
Aneurysm9 reviewed Mar 18, 2022
View reviewed changes
pkg/translator/prometheusremotewrite/go.mod Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@dashpole dashpole force-pushed the target_info_exporter branch from 91ab9c8 to ead102f Compare March 20, 2022 14:44
codeboten
codeboten reviewed Apr 4, 2022
View reviewed changes
Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

@Aneurysm9 please review, your comments have been addressed.

Aneurysm9
Aneurysm9 approved these changes Apr 4, 2022
View reviewed changes
pkg/translator/prometheusremotewrite/go.mod Outdated Show resolved Hide resolved
@dashpole dashpole force-pushed the target_info_exporter branch 2 times, most recently from 7f46614 to fb6c587 Compare April 5, 2022 17:28
@dashpole dashpole force-pushed the target_info_exporter branch from fb6c587 to 7703c88 Compare April 5, 2022 17:59
@codeboten codeboten added the ready to merge Code review completed; ready to merge by maintainers label Apr 6, 2022
@codeboten codeboten merged commit 4c48a85 into open-telemetry:main Apr 6, 2022
@ahayworth ahayworth mentioned this pull request May 16, 2022
Closed
@badjware badjware mentioned this pull request Jul 11, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codeboten codeboten codeboten left review comments

@Aneurysm9 Aneurysm9 Aneurysm9 approved these changes

Assignees

@codeboten codeboten

Labels
ready to merge Code review completed; ready to merge by maintainers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[prometheus components] Translate the "target" info metric to/from resource attributes
3 participants
@dashpole @codeboten @Aneurysm9