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

Service selection: Starting with labels besides service_name #813

Merged
merged 55 commits into from
Oct 16, 2024

Conversation

gtk-grafana
Copy link
Contributor

@gtk-grafana gtk-grafana commented Oct 2, 2024

Building on top of #801.
Fixes: #819

Allowing users to pick another label besides service_name on the service selection scene

Notes:

  • Removing aggregated metrics from volume api calls
  • Refactoring code "service" related, to use primaryLabel, primaryLabelValue instead
  • Not renaming files yet to keep down on conflicts.
image image image

gtk-grafana and others added 26 commits September 23, 2024 15:09
Base automatically changed from gtk-grafana/service-not-required-poc to main October 3, 2024 13:37
@gtk-grafana gtk-grafana changed the title WIP: Service selection -> Primary label selection Service selection: Starting with labels besides service_name Oct 9, 2024
@gtk-grafana gtk-grafana self-assigned this Oct 10, 2024
@gtk-grafana gtk-grafana added this to the 1.0.2 milestone Oct 10, 2024
@gtk-grafana gtk-grafana marked this pull request as ready for review October 10, 2024 17:08
@gtk-grafana gtk-grafana requested a review from a team as a code owner October 10, 2024 17:08
Copy link
Contributor

@svennergr svennergr left a comment

Choose a reason for hiding this comment

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

Somehow the label count in the tabs is not aligned to the values once a tab/label is selected:
image

I think we should rename this placeholder to Search values:
image

Can we remove the Loading services... copy?

Screen.Recording.2024-10-11.at.11.24.32.mov

I really like the feature, great job. Something that we discussed with @zizzpudding at one point was to add + buttons to label values next to the Select button. I think that would bring the biggest win here.
By using that + button, you can select a first set of labels/values at the starting page, and then click Select to navigate away from the "ServiceSelection" page.
I don't think that's something that necessarily need to go into this PR, but something that we should definitely iterate on.

@gtk-grafana
Copy link
Contributor Author

@svennergr

Somehow the label count in the tabs is not aligned to the values once a tab/label is selected:

This appears to be a discrepancy in counts between volume and detected_labels.

detected_labels:
https://ops.grafana-ops.net/api/datasources/uid/OP27Xzxnk/resources/detected_labels?query=&start=2024-10-15T12%3A12%3A41.535Z&end=2024-10-15T12%3A42%3A41.535Z

Volume
https://ops.grafana-ops.net/api/datasources/uid/OP27Xzxnk/resources/index/volume?query=%7Bbatch_kubernetes_io_job_name%3D~%60.%2B%60%7D&start=2024-10-15T12%3A12%3A41.535Z&end=2024-10-15T12%3A42%3A41.535Z

Look at batch_kubernetes_io_job_name, detected_labels says cardinality is 3047, but we only get 100 results back from volume.

CC: @trevorwhitney

@gtk-grafana
Copy link
Contributor Author

gtk-grafana commented Oct 15, 2024

Is the above just that volume only returns results when the values are not empty?
Since the options were remove any cardinality counts from the tabs or deal with the discrepancy we opted to add a tooltip next to the logs volume count. We can revisit this if it turns out to be confusing or distracting to users.

image

@gtk-grafana gtk-grafana requested review from svennergr and a team October 15, 2024 15:10
Copy link
Contributor

@svennergr svennergr left a comment

Choose a reason for hiding this comment

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

Just a few nits, but overall LGTM!

src/services/filters.ts Outdated Show resolved Hide resolved
src/services/text.ts Outdated Show resolved Hide resolved
@gtk-grafana gtk-grafana enabled auto-merge (squash) October 16, 2024 12:05
@gtk-grafana gtk-grafana merged commit 0991160 into main Oct 16, 2024
4 checks passed
@gtk-grafana gtk-grafana deleted the gtk-grafana/service-selection-any-label-tabs branch October 16, 2024 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Service selection: Starting with labels besides service_name
2 participants