[processor/groupbyattrsprocessor] allow empty keys for compaction

[pmm-sumo]

Description:

Extension of groupbyattrsprocessor for compacting data when spread across multiple ResourceSpans/ResourceMetrics/ResourceLogs with matching Resource and InstrumentationLibrary

Link to tracking Issue: #2265 in core

Testing: Several unit tests added

Documentation: Docs clarified on usage of empty keys. Provided example on compaction

Benchmark results (compacting 100 spans in different layouts):

BenchmarkCompacting
BenchmarkCompacting/instrumentation_library_count=1,_spans_per_library_count=100
BenchmarkCompacting/instrumentation_library_count=1,_spans_per_library_count=100-16         	   27966	     42352 ns/op
BenchmarkCompacting/instrumentation_library_count=10,_spans_per_library_count=10
BenchmarkCompacting/instrumentation_library_count=10,_spans_per_library_count=10-16         	   23912	     50266 ns/op
BenchmarkCompacting/instrumentation_library_count=100,_spans_per_library_count=1
BenchmarkCompacting/instrumentation_library_count=100,_spans_per_library_count=1-16         	   16819	     71327 ns/op

[pkositsyn]

I have 2 insights:

1. There is a comment about prohibiting empty keys in config.go, it should be changed as well
2. I have an example of Jaegerexporter, which sends different ResourceSpans independently in separate calls. Given that, I feel like in many cases setting batchpeocessor -> groupbyattrs(with empty set of keys) -> jaeger exporter will benefit performance a lot. Might be a good idea to make performance test and add something to jaeger exporter docs as well

[pkositsyn]

I have done some tests with adding groupbyattrs with empty keys for jaeger exporter

On the following screenshots 2 versions are compared: first window without groupbyattrs, and second with this processor.

Spans rate in jaeger exporter

CPU for collector

The benchmark was conducted via tracegen docker-compose, adding this diff to current branch

diff --git a/examples/tracegen/docker-compose.yml b/examples/tracegen/docker-compose.yml
index 015068b5e..d6842c6ce 100644
--- a/examples/tracegen/docker-compose.yml
+++ b/examples/tracegen/docker-compose.yml
@@ -5,7 +5,16 @@ services:
     image: jaegertracing/all-in-one:latest
     ports:
       - "16686:16686"
-      - "14250"
+      - "14250" 
+
+  prometheus:
+    image: prom/prometheus
+    ports:
+      - "9090:9090"
+    volumes:
+      - ./prometheus-config.yml:/etc/prometheus/prometheus.yml
+    depends_on:
+      - otel-collector
 
   otel-collector:
     build:
@@ -25,8 +34,11 @@ services:
       - otel-collector:4317
       - -otlp-insecure
       - -rate
-      - "1"
+      - "1000"
+      - -workers
+      - "4"
       - -duration
-      - 10000000s
+      - 600s
     depends_on:
       - otel-collector
+      - prometheus
diff --git a/examples/tracegen/otel-collector-config.yml b/examples/tracegen/otel-collector-config.yml
index e07abfae1..c3518e0ae 100644
--- a/examples/tracegen/otel-collector-config.yml
+++ b/examples/tracegen/otel-collector-config.yml
@@ -12,10 +12,13 @@ exporters:
 
 processors:
   batch:
+  groupbytrace:
+  groupbyattrs:
+    keys: []
 
 service:
   pipelines:
     traces:
       receivers: [otlp]
       exporters: [jaeger, logging]
-      processors: [batch]
+      processors: [groupbytrace, batch, groupbyattrs]

I added groupbytrace to split (possibly) sent batches by tracegen and make all pdata.ResourceSpans contain only one span. (Actually it is a common situation when groupbytrace is set before batching). The runs on screenshots differ only by adding/removing groupbyattrs from the pipeline

[pmm-sumo]

@pkositsyn @jpkrohling I updated README, added some test cases and examples. Let me know what do you think

[jpkrohling]

I'll add this to my review queue and should provide some feedback soon.

[jpkrohling]

So, basically, there wasn't a change in the processing itself, only on the constraint checking right? I like it :-)

Because this feels like an esoteric feature, I recommend documenting it well, including when and how it should be used and when/how it should not be used.

[jpkrohling]

To me, the appalling aspect of this feature is to get better performance while sending data out. Without calling this out explicitly, people might not realize that the advantages are good on the transport side as well.

[pmm-sumo]

If I understand correctly, this will reduce the size of the message for many of the formats (e.g. OTLP). In some cases (Jaeger) this will also reduce the number of RPC calls (since the Jaeger model maps one batch to one ResourceSpans) - if I got it right. Perhaps it would be worth calling out in jaegerexporter that groupbyattrs is recommended (or worth considering)?

[pmm-sumo]

Benchmark results (compacting 100 spans in different layouts):

BenchmarkCompacting
BenchmarkCompacting/instrumentation_library_count=1,_spans_per_library_count=100
BenchmarkCompacting/instrumentation_library_count=1,_spans_per_library_count=100-16         	   27966	     42352 ns/op
BenchmarkCompacting/instrumentation_library_count=10,_spans_per_library_count=10
BenchmarkCompacting/instrumentation_library_count=10,_spans_per_library_count=10-16         	   23912	     50266 ns/op
BenchmarkCompacting/instrumentation_library_count=100,_spans_per_library_count=1
BenchmarkCompacting/instrumentation_library_count=100,_spans_per_library_count=1-16         	   16819	     71327 ns/op

[pmm-sumo]

@jpkrohling @pkositsyn I put the note on batch processor in this section, also updated the wording and included it in the examples. If you have any suggestions how to express this better, you are more than welcome :)

[jpkrohling]

Looks good to me!

[pmm-sumo]

Thank you @jpkrohling! Just rebased