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

Prometheus: Disable hasLabelsMatchAPISupport for Thanos #68801

Closed
wants to merge 1 commit into from

Conversation

ye-will
Copy link

@ye-will ye-will commented May 22, 2023

What is this feature?

[Add a brief description of what the feature or update does.]

Why do we need this feature?

[Add a description of the problem the feature is trying to solve.]

Who is this feature for?

[Add information on what kind of user the feature is for.]

Which issue(s) does this PR fix?:

As mentioned in issue thanos-io/thanos#6338 & thanos-io/thanos#3639,

/api/v1/label/<label_name>/values does not work as expected right now.

We should disable hasLabelsMatchAPISupport() for Thanos.

Fixes #

Special notes for your reviewer:

Please check that:

  • It works as expected from a user's perspective.
  • If this is a pre-GA feature, it is behind a feature toggle.
  • The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

@ye-will ye-will requested a review from a team as a code owner May 22, 2023 11:22
@grafanabot grafanabot added area/frontend datasource/Prometheus pr/external This PR is from external contributor labels May 22, 2023
@itsmylife
Copy link
Contributor

LGTM. @gtk-grafana could you please take a look at as you have more insight about this?

@itsmylife itsmylife changed the title [datasource][prometheus] disable hasLabelsMatchAPISupport for Thanos Prometheus: Disable hasLabelsMatchAPISupport for Thanos May 23, 2023
@itsmylife itsmylife added this to the 10.0.x milestone May 23, 2023
@grafanabot
Copy link
Contributor

Hello @itsmylife!
Backport pull requests need to be either:

  • Pull requests which address bugs,
  • Urgent fixes which need product approval, in order to get merged,
  • Docs changes.

Please, if the current pull request addresses a bug fix, label it with the type/bug label.
If it already has the product approval, please add the product-approved label. For docs changes, please add the type/docs label.
If the pull request modifies CI behaviour, please add the type/ci label.
If none of the above applies, please consider removing the backport label and target the next major/minor release.
Thanks!

@gtk-grafana
Copy link
Contributor

@ye-will It looks like Thanos is still planning to address this issue, instead of Grafana forcing all users with Thanos versions selected to use the /series endpoints, users can select a compatibility version (or just not select any version), and then get the fallback API calls, that way when Thanos is able to address this bug people can start using the new functionality without waiting for Grafana to implement and ship another fix?

@ye-will
Copy link
Author

ye-will commented May 24, 2023

@gtk-grafana The original discussion for this issue was opened about two years ago and closed stalely, I'm not sure when it would be resolved. If Thanos fixed it in a future version, we still need to change the code to satisfy with the right version. For ones who using new versions of Grafana with older versions of Thanos, we have to advise them to not select the Prometheus Type in someway, the easiest one is just disable it in the code.

@gtk-grafana
Copy link
Contributor

gtk-grafana commented May 24, 2023

@ye-will I'll reach out to the Thanos maintainers and see if we can get an update from them as to the status of those issues. If Thanos doesn't support the match[] parameter on the label/.../values then Grafana & Thanos should do a better job of documenting that.

@gtk-grafana
Copy link
Contributor

I reached out via the CNCF Thanos slack channel, but I have yet to hear a response. I'll try to check back in with them this week.

@gtk-grafana
Copy link
Contributor

My request was acked: https://cloud-native.slack.com/archives/CK5RSSC10/p1685018637397729, but still waiting on response

@grafanabot
Copy link
Contributor

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 2 weeks if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@grafanabot grafanabot added stale Issue with no recent activity and removed stale Issue with no recent activity labels Jul 10, 2023
@github-actions
Copy link
Contributor

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 2 weeks if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions bot added the stale Issue with no recent activity label Aug 11, 2023
@github-actions
Copy link
Contributor

This pull request has been automatically closed because it has not had activity in the last 2 weeks. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions bot closed this Aug 25, 2023
@zerok zerok modified the milestones: 10.0.x, 10.0.5 Sep 5, 2023
@GiedriusS
Copy link
Contributor

Fixed in Thanos in thanos-io/thanos#6816.

@itsmylife
Copy link
Contributor

@GiedriusS Thanks for pinging us. Much appreciated!
@gtk-grafana This is fixed in Thanos. Is there anything we should do right now? I know you said we should have something better for version-checking but just want to update my information.

@gtk-grafana
Copy link
Contributor

Nope, this issue can be closed out as the bug was fixed in Thanos.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/frontend backport v10.0.x datasource/Prometheus missing-labels pr/external This PR is from external contributor stale Issue with no recent activity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants