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

pyroscope.scrape: do not drop __meta labels #2710

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

korniltsev
Copy link
Contributor

PR Description

Modify pyroscope.scrape to not drop meta labels
Update docs for pyroscope.write to clarify what labels are dropped and preserved

Which issue(s) this PR fixes

Fixes #2629

Notes to the Reviewer

PR Checklist

  • CHANGELOG.md updated
  • Documentation added
  • Tests updated
  • Config converters updated

@korniltsev korniltsev requested review from a team and clayton-cornell as code owners February 13, 2025 04:31
Copy link
Contributor

Comment on lines +13 to +15
`pyroscope.write` receives performance profiles from other components and forwards them to a series of user-supplied endpoints using [Pyroscope' Push API](/oss/pyroscope/). When forwarding profiles, most labels starting with double underscore (`__`) are dropped before sending the data, with the following exceptions:
- `__name__` - preserved as it identifies the profile type
- `__delta__` - preserved as it's required for delta profiles
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
`pyroscope.write` receives performance profiles from other components and forwards them to a series of user-supplied endpoints using [Pyroscope' Push API](/oss/pyroscope/). When forwarding profiles, most labels starting with double underscore (`__`) are dropped before sending the data, with the following exceptions:
- `__name__` - preserved as it identifies the profile type
- `__delta__` - preserved as it's required for delta profiles
`pyroscope.write` receives performance profiles from other components and forwards them to a series of user-supplied endpoints using [Pyroscope' Push API](/oss/pyroscope/).
When `pyroscope.write` forwards profiles, most labels starting with double underscore (`__`) are dropped before the data is sent, with the following exceptions:
* `__name__` is preserved because it identifies the profile type.
* `__delta__`is preserved because it's required for delta profiles.

The link to /oss/pyroscope lands on https://grafana.com/oss/pyroscope/ Is this really where we want to direct people? Or shoudl we maybe point people at the API docs instead? Maybe here https://grafana.com/docs/pyroscope/latest/reference-server-api/ instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree we should put a better link to push API, however I'm afraid we don't have the doc for the push API or at least I'm not aware of such docs.

The link you shared is not Push API, but Ingest API which is doing the same thing, just with different APIS.

The API itself is here https://github.com/grafana/pyroscope/tree/main/api maybe we can link to the source code at least.

Or remove "Push API" mentioning and the link alltogether

Copy link
Contributor

@clayton-cornell clayton-cornell Feb 14, 2025

Choose a reason for hiding this comment

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

If the user doesn't have any direct interaction with the endpoints - and it reads that way - then I'd just remove it and leave the sentence as
`pyroscope.write` receives performance profiles from other components and forwards them to a series of user-supplied endpoints.

@clayton-cornell clayton-cornell added the type/docs Docs Squad label across all Grafana Labs repos label Feb 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/docs Docs Squad label across all Grafana Labs repos
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pyrosope.scrape, pyroscope.write: clarify dropped labels behavior
2 participants