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

feat: API: Expose optional label matcher for label names API #11982

Merged
merged 6 commits into from
Jun 3, 2024
Merged

feat: API: Expose optional label matcher for label names API #11982

merged 6 commits into from
Jun 3, 2024

Conversation

yuri-rs
Copy link
Contributor

@yuri-rs yuri-rs commented Feb 18, 2024 •
edited
Loading

What this PR does / why we need it:
This is an enhancement to add the optional matchers on the label names API

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:
This change would be very useful for the Grafana enhancement grafana/grafana#74432 that I'm working on.
Most of the changes just duplicate LabelValuesForMetricName implementation and it looks pretty straightforward.

Checklist

  • Reviewed the CONTRIBUTING.md guide (required)
  • Documentation added
  • Tests updated
  • CHANGELOG.md updated
    • If the change is worth mentioning in the release notes, add add-to-release-notes label
  • Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
  • For Helm chart changes bump the Helm chart version in production/helm/loki/Chart.yaml and update production/helm/loki/CHANGELOG.md and production/helm/loki/README.md. Example PR
  • If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

@yuri-rs yuri-rs requested a review from a team as a code owner February 18, 2024 07:06
@CLAassistant
Copy link

CLAassistant commented Feb 18, 2024 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added the type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories label Feb 18, 2024
@yuri-rs
Copy link
Contributor Author

yuri-rs commented Feb 19, 2024

@periklis this is almost a copy of #8824
Could you please take a look and share your opinion?

@periklis
Copy link
Collaborator

@yuri-rs AFAIU the grafana/grafana/pull/83044 enhancement speaks about support the query selector on the label values API. I don't quite get the point on why we need the selector on the label names API too?!? I might miss something conceptual or simply misread your Grafana PR, please help me sort out why we need this change.

@yuri-rs
Copy link
Contributor Author

yuri-rs commented Feb 21, 2024

@periklis The Grafana enhancement concerns label values, true, but why not extend the same enhancement to label names as well?
For Grafana it would be simpler and more consistent if both APIs supported the match[] selector.

It would also be much better for network efficiency. Using the /labels API with a selector could allow us to retrieve perhaps only 100 label names, as opposed to extracting thousands of series with /series.

@periklis
Copy link
Collaborator

@periklis The Grafana enhancement concerns label values, true, but why not extend the same enhancement to label names as well? For Grafana it would be simpler and more consistent if both APIs supported the match[] selector.

It would also be much better for network efficiency. Using the /labels API with a selector could allow us to retrieve perhaps only 100 label names, as opposed to extracting thousands of series with /series.

Sure can you provide practical examples on how the Grafana tool would make use of it today? Does it support similar matchers on ther data sources?

@yuri-rs
Copy link
Contributor Author

yuri-rs commented Feb 21, 2024

Sure can you provide practical examples on how the Grafana tool would make use of it today? Does it support similar matchers on ther data sources?

Sure. '/labels' API with matchers works fine for Prometheus datasources, that supports this API:
image

@yuri-rs
Copy link
Contributor Author

yuri-rs commented Mar 21, 2024

Hi @periklis,
Do you have any further comments?
Are there any remaining steps that you would recommend to finalize this update?
Your insights would be greatly appreciated.
Thank you!

@periklis
Copy link
Collaborator

@cyriltovena WDYT?

cyriltovena
cyriltovena approved these changes May 27, 2024
View reviewed changes
Copy link
Contributor

@cyriltovena cyriltovena left a comment

Choose a reason for hiding this comment

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

LGTM !

Thank you for this and so sorry it got lost in the way.

@cyriltovena
Copy link
Contributor

Please rebase and ping me again to merge this.

@yuri-rs yuri-rs changed the title API: Expose optional label matcher for label names API feat: API: Expose optional label matcher for label names API May 27, 2024
@yuri-rs
Copy link
Contributor Author

yuri-rs commented May 27, 2024

@cyriltovena Thanks for the review. I have rebased the PR and remove changes from CHANGELOG.md.
Also I changed the PR summary to "feat: ..."
Is it correct way to handle changelog now? or I should do something different?

Also please check the PR before merge since some tests are failing for me because of #9081

@yuri-rs
API: Expose optional label matcher for label names API
b1e966c 
This is an enhancement to add the optional matchers on the label names API
@yuri-rs
Copy link
Contributor Author

yuri-rs commented Jun 2, 2024

@cyriltovena Could you please take a look again?

@cyriltovena cyriltovena enabled auto-merge (squash) June 3, 2024 14:28
@cyriltovena cyriltovena merged commit 8084259 into grafana:main Jun 3, 2024
59 of 60 checks passed
trevorwhitney added a commit that referenced this pull request Jun 3, 2024
@trevorwhitney
Squashed commit of the following:
9c6a884 
commit 35585db
Merge: 1822b88 47f0236
Author: Trevor Whitney <[email protected]>
Date:   Mon Jun 3 11:43:12 2024 -0600

    Merge branch 'main' into sample-count-and-bytes

commit 1822b88
Author: Trevor Whitney <[email protected]>
Date:   Mon Jun 3 11:42:52 2024 -0600

    fix: formatting

commit 47f0236
Author: Dylan Guedes <[email protected]>
Date:   Mon Jun 3 14:41:55 2024 -0300

    feat: Introduce `index audit` to `lokitool` (#13008)

    Adds a new `index audit` command to the `lokitool` cmd.
    The new `index audit` validates that all chunks required by a given index are available at the object storage. This is useful to validate if you're missing data after a backfill or when migrating data from one Loki instance to another.
    See `pkg/tool/audit/README.md` for usage instructions.

commit 71507a2
Author: Kaviraj Kanagaraj <[email protected]>
Date:   Mon Jun 3 18:18:39 2024 +0200

    feat(canary): Add test to check query results with and without cache. (#13104)

    Signed-off-by: Kaviraj <[email protected]>

commit 7942e57
Merge: 6ed195e 8084259
Author: Trevor Whitney <[email protected]>
Date:   Mon Jun 3 09:27:58 2024 -0600

    Merge branch 'main' into sample-count-and-bytes

commit 6ed195e
Author: Trevor Whitney <[email protected]>
Date:   Mon Jun 3 09:25:37 2024 -0600

    fix: nanosecond values in test with non-decimal seconds value

commit 8084259
Author: Yuri Kotov <[email protected]>
Date:   Mon Jun 3 21:30:25 2024 +0700

    feat: API: Expose optional label matcher for label names API (#11982)

    Co-authored-by: Cyril Tovena <[email protected]>

commit 09faea8
Author: Yoshitaka Fujii <[email protected]>
Date:   Mon Jun 3 21:16:10 2024 +0900

    docs: Fix link in examples (#13094)

    Co-authored-by: J Stickler <[email protected]>

commit c8cc0fb
Author: Grot (@grafanabot) <[email protected]>
Date:   Mon Jun 3 12:39:26 2024 +0100

    chore( operator): community release 0.6.1 (#12593)

commit fbd2739
Author: Joao Marcal <[email protected]>
Date:   Mon Jun 3 12:15:47 2024 +0200

    chore(operator): prepare community release v0.6.1 (#13105)

commit 4f3ed77
Author: Robert Jacob <[email protected]>
Date:   Mon Jun 3 11:02:15 2024 +0200

    fix(operator): Use a minimum value for replay memory ceiling (#13066)

commit cbf9fc0
Author: Trevor Whitney <[email protected]>
Date:   Fri May 31 16:46:39 2024 -0600

    docs: update docs

commit 29febb7
Author: Trevor Whitney <[email protected]>
Date:   Fri May 31 16:42:12 2024 -0600

    chore: make format

commit 87f7282
Author: Trevor Whitney <[email protected]>
Date:   Fri May 31 16:34:48 2024 -0600

    chore: clean up linting

commit abb31a8
Merge: 33ead60 00d3c7a
Author: Trevor Whitney <[email protected]>
Date:   Fri May 31 16:10:58 2024 -0600

    Merge branch 'main' into sample-count-and-bytes

commit 33ead60
Author: Trevor Whitney <[email protected]>
Date:   Fri May 31 16:03:04 2024 -0600

    feat: hook up samples endpoint

commit eb84303
Author: Trevor Whitney <[email protected]>
Date:   Fri May 31 13:07:49 2024 -0600

    chore: a bit of cleanup

commit 6dd77ae
Author: Trevor Whitney <[email protected]>
Date:   Fri May 31 12:56:05 2024 -0600

    feat: refactor metric samples to be it's own endpoint

commit 2587657
Author: Trevor Whitney <[email protected]>
Date:   Fri May 24 17:35:15 2024 -0600

    fix: grouping

commit b897fc5
Author: Trevor Whitney <[email protected]>
Date:   Fri May 24 13:36:00 2024 -0600

    fix: ring proxy methods on pattern ring_client

commit 0bfd0ad
Merge: 68aa188 efdae3d
Author: Trevor Whitney <[email protected]>
Date:   Thu May 23 17:04:32 2024 -0600

    Merge branch 'main' into sample-count-and-bytes

commit 68aa188
Author: Trevor Whitney <[email protected]>
Date:   Thu May 23 17:03:32 2024 -0600

    feat: guard aggregation behavior behind a feature flag

commit f0d6a92
Author: Trevor Whitney <[email protected]>
Date:   Thu May 23 14:03:32 2024 -0600

    feat: reject filter queries to /patterns endpoint

commit dc620e7
Author: Trevor Whitney <[email protected]>
Date:   Wed May 8 14:08:44 2024 -0600

    feat: collect and serve pre-agg bytes and count

    * pre-aggregate bytes and count per stream in the pattern ingester
    * serve bytes_over_time and count_over_time queries from the patterns
      endpoint
@yuri-rs yuri-rs deleted the api/label-with-match branch June 4, 2024 02:40
@loki-gh-app loki-gh-app bot mentioned this pull request Jun 10, 2024
Closed
This was referenced Jun 17, 2024
Merged
Closed
@loki-gh-app loki-gh-app bot mentioned this pull request Jul 1, 2024
Closed
@grafanabot grafanabot mentioned this pull request Jul 2, 2024
Open
This was referenced Jul 3, 2024
Closed
Closed
Closed
@RodrigoCMoraes RodrigoCMoraes mentioned this pull request Jul 12, 2024
Closed
@loki-gh-app loki-gh-app bot mentioned this pull request Jul 15, 2024
Closed
@loki-gh-app loki-gh-app bot mentioned this pull request Jul 22, 2024
Closed
@loki-gh-app loki-gh-app bot mentioned this pull request Oct 18, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cyriltovena cyriltovena cyriltovena approved these changes

Assignees
No one assigned
Labels
size/L type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@yuri-rs @CLAassistant @periklis @cyriltovena