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

chore: Ensure presence of service_name label #745

Merged
merged 6 commits into from
Jun 20, 2023

Conversation

korniltsev
Copy link
Collaborator

@@ -167,7 +169,15 @@ func populateLabels(lset labels.Labels, cfg ScrapeConfig) (res, orig labels.Labe
lb.Set(model.InstanceLabel, addr)
}

if serviceName := lset.Get(phlaremodel.LabelNameServiceName); serviceName == "" {
if serviceName = lset.Get(phlaremodel.LabelNameServiceNameK8s); serviceName == "" {
return nil, nil, errors.New("service_name label is not specified")
Copy link
Member

@petethepig petethepig Jun 6, 2023

Choose a reason for hiding this comment

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

So if I don't have either of these it will throw an error on ingestion scraping, right?

Maybe this was discussed already and there's consensus, in which case ignore me, but I would prefer we default to some values like "unspecified" or "default" or something like that.

Copy link
Collaborator

@cyriltovena cyriltovena 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 think we might want to also add a pyrosscope to our default config file which scrape itself WDYT ?

@cyriltovena cyriltovena enabled auto-merge (squash) June 20, 2023 07:05
@cyriltovena cyriltovena merged commit 94fde6b into grafana:main Jun 20, 2023
17 checks passed
simonswine pushed a commit to simonswine/pyroscope that referenced this pull request Jun 30, 2023
* chore: Ensure presence of service_name label in grafana agent https://github.com/grafana/phlare/issues/711

fix tests

* lint

* infer service_name

* fix

* Fixes the config example

---------

Co-authored-by: Cyril Tovena <cyril.tovena@gmail.com>
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

3 participants