open-telemetry / opentelemetry-collector-contrib

Contrib repository for the OpenTelemetry Collector
https://opentelemetry.io
Apache License 2.0
2.92k stars 2.29k forks source link

awskinesisexporter: Add support for KPL aggregation #6094

Open MovieStoreGuy opened 2 years ago

MovieStoreGuy commented 2 years ago

Is your feature request related to a problem? Please describe. With the most recent updates into the exporter, KPL aggregation was one thing that was dropped and I would like to see come back.

Describe the solution you'd like Can implement this: https://docs.aws.amazon.com/streams/latest/dev/kinesis-kpl-concepts.html#kinesis-kpl-concepts-aggretation

Describe alternatives you've considered The current method limits how much data most can be sent however, it can be optimised to include more per batch using the PutRecords api.

So instead of using splitting an incoming resource slice with one resource per batch, the exporter should be able to add multiple resources per record.

mottibec commented 2 years ago

Is anyone working on this? If not, I would like to. I was thinking about using an existing lib for the KPL aggregation. It looks like aws don't have an official client to support this in go, but I ran into https://github.com/a8m/kinesis-producer.

jpkrohling commented 2 years ago

It's yours, @mottibec. Not sure about the library: if really necessary, we can certainly consider it, but it will make the dep tree more complex.

cc @anuraaga

MovieStoreGuy commented 2 years ago

Hey @mottibec,

I had purposely avoided using that library for a few reasons:

We had tried it for a period of time internally at Atlassian and found the max throughput we could achieve was a batch size of 1 and even then it was problematic due to how large payloads could be following the semantic convention for traces and an internally defined one for metrics.

Once we moved over to the current exporter, we found the number of successful writes increase on smaller streams since each message had a unique (not the same) partition key and retries, timeouts, and queueing could all be implemented by the existing export helpers within the project.

mottibec commented 2 years ago

@MovieStoreGuy So, if I understand you correctly, you suggest not using KPL aggregation at all and sending batch requests with the current exporter?

The primary motivation for us at aspecto to use the KPL is for computability with other components in our pipeline. At the moment, we are stuck on an old version of the collector with the kinesis exporter that supports KPL aggregation. But if it won't increase our throughput because of the payload size, it might be worth refactoring the other components in our pipeline instead and using the exporter as-is.

MovieStoreGuy commented 2 years ago

@MovieStoreGuy So, if I understand you correctly, you suggest not using KPL aggregation at all and sending batch requests with the current exporter?

Not exactly, more saying that the reference library is something that we at Atlassian spiked and found problematic hence making the custom client in the collector today.

KPL aggregation can be completely possible, just requires a bit of effort to achieve since the exporter will attempt to write the smallest atomic representation of the telemetry per given record; it can be done, and I encourage doing so, just yet to find the time to update the component to do so (partially time management, partially allowing others on my team to contribute to the project) or drop in lib that works well with the processing model used in the collector.

github-actions[bot] commented 1 year ago

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

github-actions[bot] commented 1 year ago

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

github-actions[bot] commented 8 months ago

Pinging code owners for exporter/awskinesis: @Aneurysm9 @MovieStoreGuy. See Adding Labels via Comments if you do not have permissions to add labels yourself.