[exporter/azuremonitor] support sending to multiple azuremonitor exporter

[hgaol]

Description

currently, azuremonitor exporter doesn't support sending to multiple application insights. This PR will add this feature.

Link to tracking issue

Fixes #34188

Testing

Documentation

No additional configs.

[hgaol]

++ @pcwiese to help to review, thx!

[atoulme]

you cannot assume the same Logger is passed in for all. You might need to change this logic.

[hgaol]

Thanks for the careful review! Yes, it's better to use different logger, but the appinsights SDK only has one global diagnostics writer. If adding each logger to listener, there'll be dup messages. I don't know any elegant way to solve this issue at this point. any suggestion?

https://github.com/microsoft/ApplicationInsights-Go/blob/c063db145320a271b46e51eef7bceb88176b82c1/appinsights/diagnostics.go#L32C1-L33C52

// The one and only diagnostics writer.
var diagnosticsWriter = &diagnosticsMessageWriter{}

[atoulme]

It feels like the factory should not handle any of the channel business. The component can be reused with the sharedcomponent lib, which feels more adequate.

[hgaol]

It feels like the factory should not handle any of the channel business. The component can be reused with the sharedcomponent lib, which feels more adequate.

Currently in azuremonitorexporter, the metric, log, trace are separate exporters, and they're using the same transport channel. IMO, shared component is not suitable for such case. I plan to merge metric ,log, trace exporter into single one exporter and use shared component lib to cache it. Just like what fileexporter do. It may need more code changes as well as tests.

[hgaol]

It feels like the factory should not handle any of the channel business. The component can be reused with the sharedcomponent lib, which feels more adequate.

Currently in azuremonitorexporter, the metric, log, trace are separate exporters, and for same component they're using the same transport channel. IMO, shared component is not suitable for such case. I plan to merge metric ,log, trace exporter into single one exporter and use shared component lib to cache it. Just like what fileexporter do. It needs more code changes as well as tests.

Hi @atoulme , I've done below changes and validated locally and it can ingest into multiple appinsights successfully. pls let me know if any comments, thanks for your careful review!

• merged metric, log, trace exporters into AzureMonitorExporter, and deleted metric_exporter, log_exporter and trace_exporter.
• used sharedcomponent to cache it by component id.
• add start and shutdown to initialize and close channel.
• adjusted tests.

[hgaol]

Looks the failed checks are not caused by my change?

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[hgaol]

gently ping, anyone can help reviewing this PR? thanks!

[hgaol]

Gently ping, looks code owner hasn't noticed this PR yet, anyone from @open-telemetry/collector-contrib-approvers could help to take a look? Thanks! BTW, I'm glad to be the co-owner for this component for further triage/review to balance the load if possible.

[hgaol]

Hi @TylerHelmuth , I saw you removed waiting-for-code-owners label. I was just added as owner of this component, and I'm also owner of this PR. Is there any process different? Should I still wait for the original owner (pcwiese) to review?

[TylerHelmuth]

Ya I removed the label bc you're a code owner now and you submitted the pr. Now we need an approver to approve.

[hgaol]

Ya I removed the label bc you're a code owner now and you submitted the pr. Now we need an approver to approve.

Thanks for the explanation! I'll re-solve CI issues and let you know once done!

[hgaol]

Hi @TylerHelmuth , I've fixed CI issues and validated. could you or someone from approvers to take a look? thanks!

[atoulme]

Changes LGTM. @hgaol note those changes are extensive and I hope I am not missing anything

[hgaol]

Thanks @atoulme for your careful review! Agree that it's a big change. I've validated using both connection_string and instrumentation_key, with single or multiple application insights, looks it works well. I'll continue follow if any regression issue.