Skip to content
This repository has been archived by the owner on Jul 19, 2023. It is now read-only.

Use labels API when possible instead of Series #857

Merged
merged 11 commits into from
Jul 17, 2023
Merged

Conversation

cyriltovena
Copy link
Collaborator

Series API with {} query is expensive.

@cyriltovena
Copy link
Collaborator Author

I want to finish this and get rid of any old api too

@cyriltovena
Copy link
Collaborator Author

Alright so now we only use connect-go API. (Series, LabelNames and LabelsValues)

On top of this we also pass the current query as matchers to correctly reduce values and names for the selected workload.

})
);

export async function fetchApps(): Promise<
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Couldn't find where this is called.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's not even loaded.

@cyriltovena
Copy link
Collaborator Author

See below it shows only 4 pods of the current app.
image

@cyriltovena cyriltovena enabled auto-merge (squash) July 17, 2023 10:23
Copy link
Collaborator

@simonswine simonswine left a comment

Choose a reason for hiding this comment

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

LGTM

I have noticed that the app menur no longer closes when you select an app and profile type.

Also when selecting an app with more matches (e.g. cortex-dev-01/ingester), I am getting this when I select tags fairly often (needs like 10-20 tries)

Screenshot 2023-07-17 at 13 05 23

@cyriltovena cyriltovena merged commit a1191ed into main Jul 17, 2023
17 checks passed
@cyriltovena cyriltovena deleted the feat/use-label-api branch July 17, 2023 12:07
@simonswine
Copy link
Collaborator

I see that that was on auto merge 🙂

Unsure what you mean with the 4 pods?

I am able to show more:

Screenshot 2023-07-17 at 13 06 17

@cyriltovena
Copy link
Collaborator Author

I see that that was on auto merge 🙂

Unsure what you mean with the 4 pods?

I am able to show more:

Screenshot 2023-07-17 at 13 06 17

I meant that before it would show all pods irrespectively of the selection application.

simonswine pushed a commit to simonswine/pyroscope that referenced this pull request Jul 18, 2023
* Use labels API when possible instead of Series

* More cleanup

* Uses only connect http API.

* format

* Correctly convert query to labels matcher

* Removes todos

* Fixes tenants tests

* Fixes tenants tests

* Fixes tenants tests

* Fixes e2e tests
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants