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

Add profiles' self-scraping support #392

Merged
merged 12 commits into from
Jan 31, 2025
Merged

Add profiles' self-scraping support #392

merged 12 commits into from
Jan 31, 2025

Conversation

michaeldmitry
Copy link
Contributor

Issue

Fixes #388
Fixes #384

Solution

Add a self-scraping config if no remote parca instance is connected over self-profiling-endpoint

Drive-bys

remove observability_libs/juju_topology.py from parca_scrape.py

Testing Instructions

1.juju deploy grafana-k8s grafana --channel edge --trust
2. Deploy this parca parca0
3. Deploy another app parca1
4. juju integrate parca0 grafana:grafana-source
5. juju integrate parca1 grafana:grafana-source
6. Open Grafana UI
7. Go to parca0 source -> you should see parca0's own profiles
8. Go to parca1 source -> you should see parca1's own profiles
9. juju integrate parca0:self-profiling-endpoint parca1
10. Open Grafana UI
11. Go to parca0 source -> you should not see parca0's own profiles
12. Go to parca1 source -> you should see parca1's own profiles as well as parca0's profiles

Copy link
Contributor

@PietroPasotti PietroPasotti left a comment

Choose a reason for hiding this comment

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

some refactoring needed

src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
Copy link
Contributor

@PietroPasotti PietroPasotti left a comment

Choose a reason for hiding this comment

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

little bit of cleanup needed, rest looks ok!
Wonder if we should wait for #391 to merge first or the other way around

lib/charms/parca_k8s/v0/parca_scrape.py Outdated Show resolved Hide resolved
lib/charms/parca_k8s/v0/parca_scrape.py Outdated Show resolved Hide resolved
lib/charms/parca_k8s/v0/parca_scrape.py Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
@michaeldmitry
Copy link
Contributor Author

little bit of cleanup needed, rest looks ok! Wonder if we should wait for #391 to merge first or the other way around

let's merge #391 first

@PietroPasotti PietroPasotti self-requested a review January 27, 2025 13:16
Copy link
Contributor

@PietroPasotti PietroPasotti left a comment

Choose a reason for hiding this comment

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

couple of changes necessary

src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Show resolved Hide resolved
tests/integration/helpers.py Show resolved Hide resolved
@PietroPasotti PietroPasotti self-requested a review January 31, 2025 08:33
Copy link
Contributor

@PietroPasotti PietroPasotti left a comment

Choose a reason for hiding this comment

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

LGTM!

@michaeldmitry michaeldmitry merged commit bc7c9d6 into main Jan 31, 2025
12 checks passed
@michaeldmitry michaeldmitry deleted the TAP-255 branch January 31, 2025 10:23
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.

Scrape own profiles parca_scrape depends on outdated juju_topology charmlib
3 participants