Changing logging metrics to use starttime processor

[rafaelwestphal]

Description

The current implementation implicitly relies on the exporter to set the starttime of the metric.
This makes the behavior explicit. It is also required to move to the OTLP exporter, since it does not adjust the starttime.

Related issue

b/452074867

How has this been tested?

Current integration tests.

Checklist:

• Unit tests
  • Unit tests do not apply.
  • Unit tests have been added/modified and passed for this PR.
• Integration tests
  • Integration tests do not apply.
  • Integration tests have been added/modified and passed for this PR.
• Documentation
  • This PR introduces no user visible changes.
  • This PR introduces user visible changes and the corresponding documentation change has been made.
• Minor version bump
  • This PR introduces no new features.
  • This PR introduces new features, and there is a separate PR to bump the minor version since the last release already.
  • This PR bumps the version.

[franciscovalentecastro]

Please add a bug tracker and summarize briefly the issue.

[franciscovalentecastro]

What strategy are we using to set the start times ? Is this equivalent to the Google Cloud exporter strategy ?

[rafaelwestphal]

It uses the subtract_initial_point strategy

metricstarttime/loggingmetrics_7:
  strategy: subtract_initial_point

https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/processor/metricstarttimeprocessor/internal/subtractinitial/adjuster.go#L217-L226

This is what is done on the exporter:
https://github.com/GoogleCloudPlatform/opentelemetry-operations-go/blob/main/exporter/collector/internal/normalization/standard_normalizer.go#L250-L257

From my understanding, they both drop the first point when the starttime is zero and cache it as a reference value for the next points.

[franciscovalentecastro]

Sounds good ! Thank you! Yes, it makes sense to use that strategy as a replacement for the exporter "standard_normalization".

[franciscovalentecastro]

I have a more general question of this approach, that would also encompass the #2093 PR. Since we removed the Start Time addition in the prometheus receiver, every self metric (and prometheus metric) AFAIU is implicitly using the GCM exporter to set start times.

• Is it better (more robust, handles more edge cases, etc) to place the MetricStartTimeProcessor at the begging (after the prometheus scrape) of the pipeline or at the end (before the exporter) ?

Some details :

• I know we are attempting to place it close to the start, since we are trying to avoid a bug in the MetricStartTimeProcessor + int64, but if there was not bug, which place would be simpler and most effective ?

• If the answer is at the end we could find an alternative way to avoid the "bug" by casting all metrics to "double" at the end. WDYT ?

CC @ridwanmsharif @rafaelwestphal

[rafaelwestphal]

I have a more general question of this approach, that would also encompass the #2093 PR. Since we removed the Start Time addition in the prometheus receiver, every self metric (and prometheus metric) AFAIU is implicitly using the GCM exporter to set start times.

• Is it better (more robust, handles more edge cases, etc) to place the MetricStartTimeProcessor at the begging (after the prometheus scrape) of the pipeline or at the end (before the exporter) ?

Some details :

• I know we are attempting to place it close to the start, since we are trying to avoid a bug in the MetricStartTimeProcessor + int64, but if there was not bug, which place would be simpler and most effective ?
• If the answer is at the end we could find an alternative way to avoid the "bug" by casting all metrics to "double" at the end. WDYT ?

CC @ridwanmsharif @rafaelwestphal

Talked offline, but documenting here for posterity:

The bug is not in the processor for start times. It is actually an issue with the OTel SDK and the OTel SDK doesn't provide a good way to convert from a double to an int (both OTTL and the transform processor simply copy the as_double value to the as_int value) and downstream processors/exporters/backends see both and make a determination at that point about which it is, an INT or a DOUBLE. If we do need to apply a fix to a processor and not the SDK, I would actually argue it should happen in the modify/OTTL processors

Having the processor be added right before export is a good idea, is also more similar to what was happening in the googlecloudexporters.
The cons of putting it unilaterally are:

• extra processor for pipelines that might never need it (like hostmetrics). But I doubt there is much additional CPU/Memory over head cause this work is already being done in the current exporters
• Not needed with the googlecloudexporters are used

[rafaelwestphal]

This issue has been fixed by #2152 and this workaround is no longer needed.