Take this declarative config:
file_format: "1.1"
meter_provider:
readers:
- periodic:
exporter:
console:
temporality_preference: delta
default_histogram_aggregation: base2_exponential_bucket_histogram
Once #6954 is merged, attempting to startNodeSDK() with this declarative config will result in this diag.warn:
Config warning: some specified PushMetricExporter configuration properties were not handled by SDK setup: ["temporality_preference","default_histogram_aggregation"]
This issue is about supporting those temporality_preference and default_histogram_aggregation options.
They could be handled separately, because there are separate issues for each.
default_histogram_aggregation
temporality_preference
ConsoleMetricExporter currently accepts a temporalitySelector?: AggregationTemporalitySelector; argument.
However, some issues here:
- This
temporalitySelector is not the same type for specifying a temporality_preference config value as is currently accepted by the OTLPMetricExporter classes. They take a temporalityPreference:
|
export interface OTLPMetricExporterOptions extends OTLPExporterConfigBase { |
|
temporalityPreference?: |
|
| AggregationTemporalityPreference |
|
| AggregationTemporality; |
|
aggregationPreference?: AggregationSelector; |
|
} |
Would it be better to have ConsoleMetricExporter and OTLPMetricExporter accept the same type of option for a "temporality preference"?
- In addition to that, there is some confusion (and I think a limitation) in the
AggregationTemporalitySelector type accepted by ConsoleMetricExporter.
That type is as follows:
export enum AggregationTemporality {
DELTA,
CUMULATIVE,
}
export type AggregationTemporalitySelector = (
instrumentType: InstrumentType
) => AggregationTemporality;
That AggregationTemporality does not support the "low_memory" the value for temporality_preference defined in declarative config:
"ExporterTemporalityPreference": {
"type": [
"string",
"null"
],
"enum": [
"cumulative",
"delta",
"low_memory"
]
},
Now look at the type used for OTLPMetricExporter's:
export interface OTLPMetricExporterOptions extends OTLPExporterConfigBase {
temporalityPreference?:
| AggregationTemporalityPreference
| AggregationTemporality;
aggregationPreference?: AggregationSelector;
}
export enum AggregationTemporalityPreference {
DELTA,
CUMULATIVE,
LOWMEMORY,
}
Why does are there both enum AggregationTemporalityPreference and enum AggregationTemporality?
The latter is missing LOWMEMORY.
I haven't looked at the history here.
Do we need a breaking change to sdk-metrics to get to just one type?
Take this declarative config:
Once #6954 is merged, attempting to
startNodeSDK()with this declarative config will result in this diag.warn:This issue is about supporting those
temporality_preferenceanddefault_histogram_aggregationoptions.They could be handled separately, because there are separate issues for each.
default_histogram_aggregationFirst, one needs to add support for meaningfully passing a
default_histogram_aggregationvalue toConsoleMetricExporter.opentelemetry-js/packages/sdk-metrics/src/export/ConsoleMetricExporter.ts
Lines 13 to 15 in 636e27a
It isn't obvious to me how best to do that. For example the
OtlpGrpcMetricExportertype in declarative config (https://opentelemetry.io/docs/specs/otel-config/types/#type-otlpgrpcmetricexporter) also takes adefault_histogram_aggregation. TheOTLPMetricExporterfrom@opentelemetry/exporter-metrics-otlp-grpctakes aaggregationPreferenceargument that is built from thisdefault_histogram_aggregationvalue. (SeeaggregationSelectorFromConfigfrom fix(sdk-node)!: fail-fast on MeterProvider creation from config file #6954)Is having an
aggregationPreferenceargument toConsoleMetricExporterreasonable?Second, one needs to update the
case 'console'increatePushMetricExporterFromConfig()once fix(sdk-node)!: fail-fast on MeterProvider creation from config file #6954 is merged.temporality_preferenceConsoleMetricExporter currently accepts a
temporalitySelector?: AggregationTemporalitySelector;argument.However, some issues here:
temporalitySelectoris not the same type for specifying atemporality_preferenceconfig value as is currently accepted by theOTLPMetricExporterclasses. They take atemporalityPreference:opentelemetry-js/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/OTLPMetricExporterOptions.ts
Lines 12 to 17 in 636e27a
Would it be better to have ConsoleMetricExporter and OTLPMetricExporter accept the same type of option for a "temporality preference"?
AggregationTemporalitySelectortype accepted by ConsoleMetricExporter.That type is as follows:
That
AggregationTemporalitydoes not support the"low_memory"the value fortemporality_preferencedefined in declarative config:Now look at the type used for OTLPMetricExporter's:
Why does are there both
enum AggregationTemporalityPreferenceandenum AggregationTemporality?The latter is missing
LOWMEMORY.I haven't looked at the history here.
Do we need a breaking change to sdk-metrics to get to just one type?