search

grobian / carbon-c-relay

Enhanced C implementation of Carbon relay, aggregator and rewriter
Apache License 2.0
380 stars 107 forks source link

Fix #400 Tagged metrics dropped during aggregation #401

Closed Pavel-R closed 2 years ago

Pavel-R commented 4 years ago

Removing garbage from firstspace variable with strchr() just before sending it to router seem to me as a sufficient solution. All other checks and sanitizes was performed earlier.

grobian commented 4 years ago

What is the desired behaviour here for tagged metrics and aggregations?

Should the tags be considered as unique part of the metric?

Pavel-R commented 4 years ago

You are right. Imho, tags represents uniqueness of the value same as dot annotation in v1 of metrics. Only difference is that they may not presume the order, in which the are mentioned in a string. As for desired behavior, i'll expect that metrics from cpu load;host=ex1 will not be aggregated with cpu_load;host=ex2,but, in a same time, i'm expecting requests;host=ex1;realm=dev will be aggregated with requests;realm=dev;host=ex1. This pull request does not solve this problem, so it must be rejected. I suggest rename issue #400 to "support aggregation for tagged metrics" with label "enhancement" or so.

grobian commented 2 years ago

rejecting based on last comment