[exporter/loadbalacing] Add support for new streamID routing #32690
Conversation
RichieSams
requested review from
jpkrohling,
mx-psi and
dmitryax
as code owners
github-actions
bot
added
cmd/configschema
| * `traceID` (default): exports spans based on their `traceID`. | ||
| * If not configured, defaults to `traceID` based routing. | ||
| * The `routing_key` property is used to specify how to route values (spans or metrics) to exporters based on different parameters. This functionality is currently enabled only for `trace` and `metric` pipeline types. It supports one of the following values: | ||
| * `service`: routes values based on their service name. This is useful when using processors like the span metrics, so all spans for each service are sent to consistent collector instances for metric collection. Otherwise, metrics for the same services are sent to different collectors, making aggregations inaccurate. |
There was a problem hiding this comment.
I believe the spanmetrics processor has been deprecated, so you may want to choose a different example here.
There was a problem hiding this comment.
Fair point. I just kept the README as it was. @jpkrohling do you have any suggestions for a replacement example?
There was a problem hiding this comment.
the span metrics connector is the replacement for that, I believe the same requirement around the load balancer exists on a setup with that connector
There was a problem hiding this comment.
I started looking into this, but I cannot complete today. I also noticed that there are quite a few things that are not necessarily related to this PR: some look like simple refactorings, some are stylistic changes. Would it be possible to send those changes on a separate PR, perhaps to be merged before this one? It would make reviewing this rather large PR easier.
|
Fair point. I'll work on breaking this up into more manageable pieces. Thanks for the input |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
This PR will be broken up into smaller pieces |
**Description:** This will merge the metrics in mdB into mdA, trying to re-use resourceMetrics, scopeMetrics, and metric values as possible. This will be used to help implement the new feature for: #32513 **Link to tracking Issue:** #32513 / #32690 **Testing:** I created a unit test which tests various scenarios of how merge behavior should happen **Documentation:** The exported function is documented using standard golang style. And there are comments inside the code to explain what is going on and why --------- Co-authored-by: Ziqi Zhao <zhaoziqi9146@gmail.com>
Description: This adds a new routing option for metrics:
streamID. This routes datapoints based on theirstreamID. That's the unique hash of all it's attributes, plus the attributes and identifying information of its resource, scope, and metric dataDetailed change list:
conststring values for all the routingKey enumsstreamIDrouting, but the other routingKey methods don't need that. And would just require more work inMergeMetrics()internal/exp/metrics:MergeMetrics()mergeMetrics()NOTE: The old
ConsumeMetrics()used its own custom hashing function to create the key forresourcerouting. The new code usesidentity.OfResource()frominternal/exp/metrics/identity. The effect is the same, but the key will be different from before. So when a user updates to this code, the routing will be slightly different.Link to tracking Issue: #32513
Testing:
I added a new suite of tests for the splitting of metrics depending on the routingKey. I utilize
goldenso all input / output metric data can be declared in yaml, which is much easier to visually parse. Meaning we can have much more complicated / "real-life" data sets.I updated the benchmarks to also test different numbers of resourceMetrics and scopeMetrics. So we can see how those affect routing times.
Documentation:
I updated the README to describe the new routingKey:
metricID, and how it works