Add metrics to Kafka exporter

[dshomoye]

Add metrics otlp exporter to kafka (previously only supported traces).

• add tests for kafka metrics exporter.

Description:

• The kafka exporter now supports both traces and metrics
• Metrics is encoded as otlp proto by default (same as Traces, also the only encoding currently implemented)
• Update the configuration of the kafka exporter to accomodate traces and metrics
  • encoding key specifies encoding for either metrics or traces.
  • Add default topic for metrics as otlp_metrics.

Testing:

• Unit tests added for metrics exporter (97.1% package cov)
• Was able to deploy the demo set up(/examples/demo). Successfully received metrics and traces in a kafka consumer

Documentation:

Updated kafka exporter README to state new defaults.
Updated CHANGELOG to reflect enhancement.

---

[linux-foundation-easycla]

The committers are authorized under a signed CLA.

• ✅ Damola Shomoye (40f6300)

[codecov]

Codecov Report

Merging #1966 (d14dbb1) into master (29859f2) will increase coverage by 0.05%.
The diff coverage is 97.26%.

@@            Coverage Diff             @@
##           master    #1966      +/-   ##
==========================================
+ Coverage   91.99%   92.04%   +0.05%     
==========================================
  Files         270      270              
  Lines       15791    15844      +53     
==========================================
+ Hits        14527    14584      +57     
+ Misses        860      857       -3     
+ Partials      404      403       -1     

Impacted Files	Coverage Δ
exporter/kafkaexporter/jaeger_marshaller.go	85.71% <ø> (ø)
exporter/kafkaexporter/otlp_marshaller.go	71.42% <71.42%> (ø)
exporter/kafkaexporter/factory.go	100.00% <100.00%> (ø)
exporter/kafkaexporter/kafka_exporter.go	100.00% <100.00%> (ø)
exporter/kafkaexporter/marshaller.go	100.00% <100.00%> (ø)
exporter/otlpexporter/otlp.go	74.35% <0.00%> (+2.56%)	⬆️
translator/internaldata/resource_to_oc.go	91.54% <0.00%> (+2.81%)	⬆️
exporter/exporterhelper/metricshelper.go	100.00% <0.00%> (+5.40%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 29859f2...d14dbb1. Read the comment docs.

[github-actions]

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

[dshomoye]

Adding a comment here to bring this PR up again, @tigrannajaryan just want to confirm there is nothing else needed on my end for this to move forward?

[asuresh4]

@dshomoye - apologies for the delay in review. Mostly LGTM, I left a few comments.

@tigrannajaryan @bogdandrutu would be great if you or anyone more familiar with the kafka exporter could also take a look.

[tigrannajaryan]

LGTM, will need confirmation from @owais too and can merge.

[pavolloffay]

• kafka exporter: Rename default topic to otlp from otlp_spans

What is the motivation to change the topic name? If you proceed with rename the receiver should be updated as well.

[dshomoye]

• kafka exporter: Rename default topic to otlp from otlp_spans

What is the motivation to change the topic name? If you proceed with rename the receiver should be updated as well.

Primarily because *span is a trace-specific name; sending metrics to a *span topic seems a bit out of place.
As for updating the receiver, there hasn't been any changes for metrics support there so it wasn't updated. Is changing the topic for consistency or is there some dependency between exporter/receiver here? (my impression is that there isn't)

[jpkrohling]

Is changing the topic for consistency or is there some dependency between exporter/receiver here?

I believe we are still allowed to make breaking changes, but people might be using this exporter already and rely on the previously published name with their own consumers. Ideally, changes like these would happen in two phases:

1. announce the change, deprecating the previous value. Log a info-level message telling that this will change (unless it's explicitly set)
2. one release further, change the level to warn (unless it's explicitly set)
3. two releases further, make the switch

[dshomoye]

people might be using this exporter already and rely on the previously published name with their own consumers.

Understood, in that case, I will revert this breaking change and add the log message for now.

[pavolloffay]

@dshomoye re the topic name, isn't it a better practice to not send various types to the same topic? If so we should document this and use different default per metrics and traces.

Using the same topic name for both types will break the current kafka receiver, it will fail to unmarshall traces.

[dshomoye]

@dshomoye re the topic name, isn't it a better practice to not send various types to the same topic? If so we should document this and use different default per metrics and traces.

Using the same topic name for both types will break the current kafka receiver, it will fail to unmarshall traces.

You're absolutely right! Mixing different data types in the same topic is not good practice - even though the user can override for trace/metric.
Still, this actually resolves the breaking change issue. The default topic for traces will stay as otlp_spans and a new default topic otlp_metrics will be added for metrics. No deprecation required.

[bogdandrutu]

For me this looks good, @pavolloffay @tigrannajaryan is it ok to merge this?

[dshomoye]

Anything needed to merge this now? @pavolloffay

[github-actions]

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

[tigrannajaryan]

Merging this broke the build: https://app.circleci.com/pipelines/github/open-telemetry/opentelemetry-collector/4718/workflows/59a274c1-241d-4928-8411-2c5330c7fc47

I am going to revert to fix the build. @dshomoye after I revert please create a new PR with this changes. You will need to make some additional changes to make sure the build passes.