Skip to content

apmotel: follow APM OTel spec and prefer delta temporality#1437

Merged
carsonip merged 8 commits intoelastic:mainfrom
carsonip:otel-delta-temporality
May 16, 2023
Merged

apmotel: follow APM OTel spec and prefer delta temporality#1437
carsonip merged 8 commits intoelastic:mainfrom
carsonip:otel-delta-temporality

Conversation

@carsonip
Copy link
Member

@carsonip carsonip commented May 12, 2023

Make apmotel follow apm otel spec: https://github.com/elastic/apm/blob/main/specs/agents/metrics-otel.md#aggregation-temporality

  • Prefer delta temporality on certain instruments
  • Filter out zero values on delta temporality

Closes #1436

@carsonip carsonip force-pushed the otel-delta-temporality branch from d75abf1 to 3ad3c80 Compare May 15, 2023 11:08
Copy link
Member

@axw axw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This brings us closer to the spec https://github.com/elastic/apm/blob/main/specs/agents/metrics-otel.md#aggregation-temporality

There's also this:

For all instrument types with delta temporality, agents MUST filter out zero values before exporting. E.g. if a counter does not change since the last export, it must not be exported.

If you have time, could you take a stab at that too? Otherwise could you please open an issue?

@carsonip
Copy link
Member Author

For all instrument types with delta temporality, agents MUST filter out zero values before exporting. E.g. if a counter does not change since the last export, it must not be exported.

If you have time, could you take a stab at that too? Otherwise could you please open an issue?

Taken a stab at it, see if it makes sense to you.

Copy link
Member

@axw axw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with the panic removed. Thanks!

Copy link
Member

@axw axw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with the panic removed. Thanks!

Co-authored-by: Andrew Wilkins <axwalk@gmail.com>
@carsonip carsonip changed the title apmotel: default to delta temporality May 16, 2023
@carsonip carsonip marked this pull request as ready for review May 16, 2023 07:58
@carsonip carsonip merged commit ea8c3f6 into elastic:main May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3 participants