Protobuf-based metricfamilyprocessor doesn't handle target label collisions correctly about client_golang HOT 7 CLOSED

Oof. Just thinking about this, said functionality ought not to be in the
client library at all and be done by the consumer of it. As it stands now,
this is business logic creep. I would make a pull to remove it from the
client and add it back to the Prometheus server.
Am 07.08.2013 20:14 schrieb "juliusv" [email protected]:

AFAICS the new protobuf-based extraction processor simply overwrites the
target's base labels on a sample if the target sets a conflicting label
itself:

https://github.com/prometheus/client_golang/blob/master/extraction/metricfamilyprocessor.go#L76

...whereas the JSON-based processors avoid such collisions by adding an
exporter_ prefix to the conflicting target labels (as specified in
https://github.com/prometheus/prometheus/wiki/Automatic-Labels-and-Synthetic-Metrics):

https://github.com/prometheus/client_golang/blob/master/extraction/processor0_0_2.go#L71

This should probably be fixed for the protobuf processor as well.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/24
.

from client_golang.

Yeah, this was from the time when this batch of code was still part of the server, not the client library. I'm happy to move it out along with the other label changes.

from client_golang.

Please do.
Am 07.08.2013 22:13 schrieb "juliusv" [email protected]:

Yeah, this was from the time when this batch of code was still part of the
server, not the client library. I'm happy to move it out along with the
other label changes.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/24#issuecomment-22280298
.

from client_golang.

I'm changing my opinion on this. The processor feeds samples back into its results channel, which circumvents any subsequent code in Prometheus' retrieval layer and ends up directly in storage. This is so we can take advantage of parallelism in the scrape->storage pipeline. If we wanted to make any targetlabel-based modifications to the result samples before they end up in the storage layer, we'd need to spin up another dispatcher in Prometheus' retrieval layer which reads from one result channel, rewrites results, and writes to a second channel which feeds the final results to storage. I have a feeling that we'd prefer merging the labels in the processing layer than introducing this kind of extra complexity?

from client_golang.

I have a refactoring that will interest you that will obviate this
concern. Let me post it for you once back from Sverige.
Am 08.08.2013 12:39 schrieb "juliusv" [email protected]:

I'm changing my opinion on this. The processor feeds samples back into its
results channel, which circumvents any subsequent code in Prometheus'
retrieval layer and ends up directly in storage. This is so we can take
advantage of parallelism in the scrape->storage pipeline. If we wanted to
make any targetlabel-based modifications to the result samples before they
end up in the storage layer, we'd need to spin up another dispatcher in
Prometheus' retrieval layer which reads from one result channel, rewrites
results, and writes to a second channel which feeds the final results to
storage. I have a feeling that we'd prefer merging the labels in the
processing layer than introducing this kind of extra complexity?

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/24#issuecomment-22315378
.

from client_golang.

@juliusv Is that still an issue?

from client_golang.

This has been resolved via a series of MergeLabelIngesters in the Prometheus server.

from client_golang.