Follow up on AggregationTemporality vs. AggregationModes

[reyang]

#2357 (comment)

[alanwest]

I'm definitely struggling to more firmly develop an opinion on the naming AggregationTemporality vs AggregationModes.

AggregationTemporality seems good to me because this is a term identified in the specification. As new users are exploring .NET's SDK - perhaps with some existing familiarity with another language's SDK - the purpose of AggregationModes may not be very apparent. Though, I do want to better understand the flexibility the term AggregationModes offers:

@reyang says #2357 (comment):

Trying to be a bit creative here, how about:

• AggregationModes.CumulativeTemporality
• AggregationModes.DeltaTemporality

I personally like this, as it gives us room to introduce something like these in the future:

• AggregationModes.ExplicitReset
• AggregationModes.ImplicitReset

This is the first I've considered scenarios related to temporality beyond just cumulative and delta. These specific scenarios ExplicitReset and ImplicitReset just sound like cumulative temporality with some mechanism provided by the SDK to invoke a reset. I'd want to think more deeply about whether introducing new values to this enumeration would be part of the solution for delivering this kind of functionality.

Another question I have is if the other contexts in which this enumeration may be used will affect opinions of how it is named. I'm mainly thinking about the view API. It's my understanding that once we have the ability to configure a view's aggregation, included with this will be the ability to configure the aggregation's temporality (e.g., the Sum Aggregation can be configured with a SumType and a Temporality).

In this context, does something like new SumAggregation { AggregationMode = AggregationModes.DeltaTemporality } feel better to folks than new SumAggregation { AggregationTemporality = AggregationTemporality.Delta?

[reyang]

In this context, does something like new SumAggregation { AggregationMode = AggregationModes.DeltaTemporality } feel better to folks than new SumAggregation { AggregationTemporality = AggregationTemporality.Delta?

I was struggling with this, too! And that's why I didn't send out the PR yet 😆

[cijothomas]

new SumAggregation { Temporality = AggregationTemporality.Delta, feels most
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/datamodel.md#temporality

[reyang]

new SumAggregation { Temporality = AggregationTemporality.Delta, feels most https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/datamodel.md#temporality

How do you think about this open-telemetry/opentelemetry-specification#1200?

[alanwest]

How do you think about this open-telemetry/opentelemetry-specification#1200?

Is your thought that we'd mush together the concepts of temporality and memory mode into a single enumeration?

If so, are you thinking that this enumeration would still be equally applicable both to both the configuration of an exporter and the configuration of a view?

[reyang]

Is your thought that we'd mush together the concepts of temporality and memory mode into a single enumeration?

No, I think it'll be better if we keep a clear distinction coz these are very different concepts. However we might be able to do something to make it simple for the user.

If so, are you thinking that this enumeration would still be equally applicable both to both the configuration of an exporter and the configuration of a view?

I think some of the enum values would apply to both, some would only apply to one.

[reyang]

I guess we can close it since it's not going anywhere, and the current names seem to be fine.