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

query: Extend metadata service to include target metadata API #4449

Closed
wants to merge 3 commits into from

Conversation

philipgough
Copy link
Contributor

As requested in the review of #4438, this change extends the metadata service to include target metadata api https://prometheus.io/docs/prometheus/latest/querying/api/#querying-target-metadata.

The implementation largely follows the example set by the metric metadata PR in #3686 and in fact piggybacks on the service defined in the metadata proto that was introduced in that feature, since the two metadata APIs are closely related.

This will avoid the need to add an additional flag to define metadata stores. This makes it more convenient but maybe less desirable (we could not take them out of experimental mode independently for example) and I am happy to introduce an additional flag and rpc service if required.

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

Extend the metadata rpc service

Verification

None required, updates proto and generated code

@philipgough philipgough force-pushed the issue-3870-1 branch 2 times, most recently from fe294de to 9351ca5 Compare July 16, 2021 09:50
message MetricMetadataInfo {
}

/// TargetMetadataInfo holds the metadata related to Target Metadata API exposed by the component.
message TargetMetadataInfo {
Copy link
Contributor

Choose a reason for hiding this comment

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

I am wondering is it better to combine this and MetricMetadataInfo to one MetadataInfo field, in the future?
As they are both belonging to metadatapb, but different services.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point, resolved

Signed-off-by: Philip Gough <philip.p.gough@gmail.com>
Signed-off-by: Philip Gough <philip.p.gough@gmail.com>
Signed-off-by: Philip Gough <philip.p.gough@gmail.com>
@stale
Copy link

stale bot commented Sep 26, 2021

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale label Sep 26, 2021
@stale stale bot closed this Oct 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants