OpenTelemetry Collector Sink Broken

[hkfgo]

Report

The first export cycle is successful, but not subsequent ones. And it's an insidious failure that tricks CI to pass :/ We'd released many Promitor versions with this bug. The latest working version is 2.8.0.

Expected Behavior

Continuous stream of metrics exported

Actual Behavior

Only the first export is successful then nothing

Steps to Reproduce the Problem

Configure an OpenTelemetry sink like documentation describes. There will either be no metrics at all or a few data points followed by nothing.

Component

Scraper

Version

Scraper-v2.11.2

Configuration

Configuration:

# Add your scraping configuration here

Logs

example

Platform

Microsoft Azure

Contact Details

No response

[github-actions]

Thank you for opening an issue! We rely on the community to maintain Promitor. (Learn more)

Is this something you want to contribute?

[hkfgo]

Here's more of a proof and some good news. I reverted #2239 in my test branch https://github.com/tomkerkhove/promitor/pull/2567/files, where the OpenTelemetryCollectorMetricSink.cs is reverted to the earliest version.

I was able to get OpenTelemetry tests to pass every time(I tried 3 times total). Feel free to have a look yourself. It's a little confusing because the scraper CI is marked red but only because logs on the OTEL Collector printed to stderr. The tests themselves all passed.

[hkfgo]

I think it's safe to say that using channels in that commit broke the sink. If you still remember, do you mind explaining what concurrency issue you were trying to fix?

Although tests pass with the revert, I did observe that the metrics reported are still missing labels. I can try incorporating changes from this past commit https://github.com/tomkerkhove/promitor/pull/2183/files#diff-3860310bca8263df9a9f5b43bd3f06fdac0975d7d85467b086550aa561883316 and see if a fully working version can be produced.

[tomkerkhove]

That's very nice finding - Thank you!

I think the issue has the details on what was going on but it was not fully thread-safe. Let's see if we can get the labels working and then we can revert that PR and move forward.

[hkfgo]

Sounds good. Let's move forward without using channels, the circle back on that concurrency issue?

[hkfgo]

I incorporated label processing logic into the above PR. The code passed CI and is running smoothly in our staging env. I'll clean up the PR a bit and it should be in good shape to merge

[tomkerkhove]

Thanks a ton, highly appreciated!

As ref: #2567

[tomkerkhove]

Fixed in #2567, thank you very much @hkfgo!

[hkfgo]

@tomkerkhove could you cut a new release for the chart as well?

[tomkerkhove]

Sure, would you mind opening a PR to update the Helm chart and package things please? I can do it but not this week; sorry

[hkfgo]

I filed an issue here for a better place to talk about it: promitor/charts#174