prometheusreceiver: Don't ignore instance labels for internal metrics

[rmfitzpatrick]

Description:
Fixing a bug - In federated endpoint scraping cases internal labels are required to differentiate scraped metrics and receiver-internal ones. These changes take the metric name into consideration when establishing "not useful" labels for metric group identity.

Link to tracking Issue:
#14453

Testing:
ongoing - I'm having issues w/ my dev environment and running these tests reliably.
Added basic unit tests for internal metric attributes.

[dashpole]

When we are scraping federated metrics, wouldn't we always want to take job + instance labels into account, not just for internal metrics?

[rmfitzpatrick]

When we are scraping federated metrics, wouldn't we always want to take job + instance labels into account, not just for internal metrics?

I believe the prometheus server target info is already preserved and made available in the resource of the scraped federated metrics. For example when I have a federated server scrape a collector's metric endpoint and* another collector scrape that federated endpoint I see:

http.scheme: http
net.host.name: target-collector
net.host.port: "8888"
service.instance.id: target-collector:8888
service.name: collector

[dashpole]

If I scrape a federated endpoint, and it has two metrics which are identical except for job + instance (non-internal metrics), they would collide and experience this issue, right? It seems fairly safe to ~always take job + instance into account.

[dashpole]

IIUC, the issue isn't that we aren't preserving job + instance (it is in the resource, as you show), but that these two metrics end up "colliding":

foo{"job": bar, "instance: "0.0.0.0:8888"}
foo{}

Right?

[rmfitzpatrick]

IIUC, the issue isn't that we aren't preserving job + instance (it is in the resource, as you show), but that these two metrics end up "colliding"< ... >
Right?

I think that's correct and am not sure why job and instance were deemed not useful. My proposed change was a minimal way to address clear collisions w/ federated scraping* but can see the value in not filtering them in any case.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: rmfitzpatrick / name: Ryan Fitzpatrick (26536bbc9f3d4bda581f4162ba8217dec4bab31c)

[dashpole]

My proposed change was a minimal way to address clear collisions w/ federated scraping* but can see the value in not filtering them in any case.

I would prefer not filtering them in any cases here.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[fatsheep9146]

any update? @rmfitzpatrick

[rmfitzpatrick]

@fatsheep9146 I've had to table this for a bit (using a global label as a workaround) but not filtering job and instance labels overall has several knock-on effects and seems to break expected resource attributes so requires a deeper awareness of the receiver than I have atm to do correctly. Will take another look with fresher eyes soon.

[bka9]

@fatsheep9146 I've had to table this for a bit (using a global label as a workaround) but not filtering job and instance labels overall has several knock-on effects and seems to break expected resource attributes so requires a deeper awareness of the receiver than I have atm to do correctly. Will take another look with fresher eyes soon.

Can you help me understand what the global label workaround is?

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[github-actions]

Closed as inactive. Feel free to reopen if this PR is still being worked on.

[shacharSirotkin]

Hi,
I'm looking for a solution for this problem.
I'm trying to federate a Prometheus and all metrics collide.

Any progress in this PR?