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

[v10.1.x] Loki: Cache extracted labels #75905

Merged
merged 1 commit into from Oct 3, 2023

Conversation

grafana-delivery-bot[bot]
Copy link
Contributor

Backport 5b63cdb from #75842


What is this feature?
Adds simple Map to cache the previous results of Loki data samples in the Loki language provider.

Why do we need this feature?
loki-data-samples queries are currently triggered directly from the Monaco completion callbacks, and not part of the react/UI application layer, so certain keystrokes in certain contexts will trigger another query. When this query takes a long time to complete, the editor UX is very sluggish and difficult to work with. Since the values returned by this query are extracted label values, which are not expected to change second-to-second, the solution proposed here is to cache requests and serve "stale" labels to prevent needing to wait for an api request whenever the user presses spacebar or comma inside a stream selector.

TL;DR;
To prevent duplicate API calls getting labels in autocomplete, we cache 2 unique query strings and their associated labels.

Who is this feature for?
Users of Loki code editor.

Which issue(s) does this PR fix?:
Fixes #75512

Special notes for your reviewer:
This is the lightest implementation I could imagine in terms of CPU/memory consumption, worried about cases where this list of labels is quite long.

We could use LRU for the added feature of smarter cache purging (instead of purging the first inserted, we'd be purging the least recently used), but that's a lot of additional overhead, and the API between map and LRU are very similar, so swapping it out would be very easy.

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.

* add simple cache to extracted label values in completion provider

(cherry picked from commit 5b63cdb)
Copy link
Contributor

@gtk-grafana gtk-grafana left a comment

Choose a reason for hiding this comment

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

Looks good, me

@gtk-grafana gtk-grafana enabled auto-merge (squash) October 3, 2023 15:47
@gtk-grafana gtk-grafana merged commit 3badf96 into v10.1.x Oct 3, 2023
19 checks passed
@gtk-grafana gtk-grafana deleted the backport-75842-to-v10.1.x branch October 3, 2023 15:50
@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2023

Backend code coverage report for PR #75905
No changes

@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2023

Frontend code coverage report for PR #75905

Plugin Main PR Difference
loki 86.24% 85.35% -.89%

@aangelisc aangelisc modified the milestones: 10.1.x, 10.1.6 Dec 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants