Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(metrics): prototype experimental advice support #3876

Merged
merged 8 commits into from
Oct 11, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/

### :rocket: (Enhancement)

* feat(metrics): prototype experimental advice support [#3876](https://github.com/open-telemetry/opentelemetry-js/pull/3876) @legendecas

### :bug: (Bug Fix)

### :books: (Refine Doc)
Expand Down
4 changes: 4 additions & 0 deletions api/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file.

## Unreleased

### :rocket: (Enhancement)

* feat(metrics): prototype experimental advice support [#3876](https://github.com/open-telemetry/opentelemetry-js/pull/3876) @legendecas

## 1.4.1

### :bug: (Bug Fix)
Expand Down
1 change: 1 addition & 0 deletions api/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export {
ObservableUpDownCounter,
UpDownCounter,
BatchObservableCallback,
MetricAdvice,
MetricAttributes,
MetricAttributeValue,
ObservableCallback,
Expand Down
14 changes: 14 additions & 0 deletions api/src/metrics/Metric.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,14 @@ import { Attributes, AttributeValue } from '../common/Attributes';
import { Context } from '../context/types';
import { BatchObservableResult, ObservableResult } from './ObservableResult';

export interface MetricAdvice {
legendecas marked this conversation as resolved.
Show resolved Hide resolved
/**
* Hint the explicit bucket boundaries for SDK if the metric is been
* aggregated with a HistogramAggregator.
legendecas marked this conversation as resolved.
Show resolved Hide resolved
*/
explicitBucketBoundaries?: number[];
}

/**
* Options needed for metric creation
*/
Expand All @@ -39,6 +47,12 @@ export interface MetricOptions {
* @default {@link ValueType.DOUBLE}
*/
valueType?: ValueType;

/**
* Status: Experimental.
* The advice influencing aggregation configuration parameters.
legendecas marked this conversation as resolved.
Show resolved Hide resolved
*/
advice?: MetricAdvice;
}

/** The Type of value. It describes how the data is reported. */
Expand Down
8 changes: 7 additions & 1 deletion packages/sdk-metrics/src/InstrumentDescriptor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import { MetricOptions, ValueType } from '@opentelemetry/api';
import { MetricAdvice, MetricOptions, ValueType } from '@opentelemetry/api';
import { View } from './view/View';

/**
Expand All @@ -38,6 +38,10 @@ export interface InstrumentDescriptor {
readonly unit: string;
readonly type: InstrumentType;
readonly valueType: ValueType;
/**
* Status: Experimental.
*/
readonly advice: MetricAdvice;
}

export function createInstrumentDescriptor(
Expand All @@ -51,6 +55,7 @@ export function createInstrumentDescriptor(
description: options?.description ?? '',
unit: options?.unit ?? '',
valueType: options?.valueType ?? ValueType.DOUBLE,
advice: options?.advice ?? {},
};
}

Expand All @@ -64,6 +69,7 @@ export function createInstrumentDescriptorWithView(
type: instrument.type,
unit: instrument.unit,
valueType: instrument.valueType,
advice: instrument.advice,
};
}

Expand Down
10 changes: 3 additions & 7 deletions packages/sdk-metrics/src/ObservableResult.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import {
} from '@opentelemetry/api';
import { AttributeHashMap } from './state/HashMap';
import { isObservableInstrument, ObservableInstrument } from './Instruments';
import { InstrumentDescriptor } from '.';

/**
* The class implements {@link ObservableResult} interface.
Expand All @@ -35,18 +34,15 @@ export class ObservableResultImpl implements ObservableResult {
*/
_buffer = new AttributeHashMap<number>();

constructor(private _descriptor: InstrumentDescriptor) {}
constructor(private _instrumentName: string, private _valueType: ValueType) {}

/**
* Observe a measurement of the value associated with the given attributes.
*/
observe(value: number, attributes: MetricAttributes = {}): void {
if (
this._descriptor.valueType === ValueType.INT &&
!Number.isInteger(value)
) {
if (this._valueType === ValueType.INT && !Number.isInteger(value)) {
diag.warn(
`INT value type cannot accept a floating-point value for ${this._descriptor.name}, ignoring the fractional digits.`
`INT value type cannot accept a floating-point value for ${this._instrumentName}, ignoring the fractional digits.`
);
value = Math.trunc(value);
}
Expand Down
5 changes: 2 additions & 3 deletions packages/sdk-metrics/src/aggregator/Drop.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@

import { HrTime } from '@opentelemetry/api';
import { AggregationTemporality } from '../export/AggregationTemporality';
import { MetricData } from '../export/MetricData';
import { InstrumentDescriptor } from '../InstrumentDescriptor';
import { MetricData, MetricDescriptor } from '../export/MetricData';
import { Maybe } from '../utils';
import { AggregatorKind, Aggregator, AccumulationRecord } from './types';

Expand All @@ -38,7 +37,7 @@ export class DropAggregator implements Aggregator<undefined> {
}

toMetricData(
_descriptor: InstrumentDescriptor,
_descriptor: MetricDescriptor,
_aggregationTemporality: AggregationTemporality,
_accumulationByAttributes: AccumulationRecord<undefined>[],
_endTime: HrTime
Expand Down
5 changes: 3 additions & 2 deletions packages/sdk-metrics/src/aggregator/ExponentialHistogram.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,10 @@ import {
import {
DataPointType,
ExponentialHistogramMetricData,
MetricDescriptor,
} from '../export/MetricData';
import { diag, HrTime } from '@opentelemetry/api';
import { InstrumentDescriptor, InstrumentType } from '../InstrumentDescriptor';
import { InstrumentType } from '../InstrumentDescriptor';
import { Maybe } from '../utils';
import { AggregationTemporality } from '../export/AggregationTemporality';
import { Buckets } from './exponential-histogram/Buckets';
Expand Down Expand Up @@ -551,7 +552,7 @@ export class ExponentialHistogramAggregator
}

toMetricData(
descriptor: InstrumentDescriptor,
descriptor: MetricDescriptor,
aggregationTemporality: AggregationTemporality,
accumulationByAttributes: AccumulationRecord<ExponentialHistogramAccumulation>[],
endTime: HrTime
Expand Down
10 changes: 7 additions & 3 deletions packages/sdk-metrics/src/aggregator/Histogram.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,13 @@ import {
Aggregator,
AggregatorKind,
} from './types';
import { DataPointType, HistogramMetricData } from '../export/MetricData';
import {
DataPointType,
HistogramMetricData,
MetricDescriptor,
} from '../export/MetricData';
import { HrTime } from '@opentelemetry/api';
import { InstrumentDescriptor, InstrumentType } from '../InstrumentDescriptor';
import { InstrumentType } from '../InstrumentDescriptor';
import { binarySearchLB, Maybe } from '../utils';
import { AggregationTemporality } from '../export/AggregationTemporality';

Expand Down Expand Up @@ -207,7 +211,7 @@ export class HistogramAggregator implements Aggregator<HistogramAccumulation> {
}

toMetricData(
descriptor: InstrumentDescriptor,
descriptor: MetricDescriptor,
Copy link
Member

Choose a reason for hiding this comment

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

Why is this change needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is needed to avoid exposing the advice on the InstrumentDescriptor to the MetricReaders and the MetricExporters.

aggregationTemporality: AggregationTemporality,
accumulationByAttributes: AccumulationRecord<HistogramAccumulation>[],
endTime: HrTime
Expand Down
9 changes: 6 additions & 3 deletions packages/sdk-metrics/src/aggregator/LastValue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,11 @@ import {
} from './types';
import { HrTime } from '@opentelemetry/api';
import { millisToHrTime, hrTimeToMicroseconds } from '@opentelemetry/core';
import { DataPointType, GaugeMetricData } from '../export/MetricData';
import { InstrumentDescriptor } from '../InstrumentDescriptor';
import {
DataPointType,
GaugeMetricData,
MetricDescriptor,
} from '../export/MetricData';
import { Maybe } from '../utils';
import { AggregationTemporality } from '../export/AggregationTemporality';

Expand Down Expand Up @@ -103,7 +106,7 @@ export class LastValueAggregator implements Aggregator<LastValueAccumulation> {
}

toMetricData(
descriptor: InstrumentDescriptor,
descriptor: MetricDescriptor,
aggregationTemporality: AggregationTemporality,
accumulationByAttributes: AccumulationRecord<LastValueAccumulation>[],
endTime: HrTime
Expand Down
9 changes: 6 additions & 3 deletions packages/sdk-metrics/src/aggregator/Sum.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,11 @@ import {
AccumulationRecord,
} from './types';
import { HrTime } from '@opentelemetry/api';
import { DataPointType, SumMetricData } from '../export/MetricData';
import { InstrumentDescriptor } from '../InstrumentDescriptor';
import {
DataPointType,
MetricDescriptor,
SumMetricData,
} from '../export/MetricData';
import { Maybe } from '../utils';
import { AggregationTemporality } from '../export/AggregationTemporality';

Expand Down Expand Up @@ -109,7 +112,7 @@ export class SumAggregator implements Aggregator<SumAccumulation> {
}

toMetricData(
descriptor: InstrumentDescriptor,
descriptor: MetricDescriptor,
aggregationTemporality: AggregationTemporality,
accumulationByAttributes: AccumulationRecord<SumAccumulation>[],
endTime: HrTime
Expand Down
7 changes: 3 additions & 4 deletions packages/sdk-metrics/src/aggregator/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@

import { HrTime, MetricAttributes } from '@opentelemetry/api';
import { AggregationTemporality } from '../export/AggregationTemporality';
import { MetricData } from '../export/MetricData';
import { InstrumentDescriptor } from '../InstrumentDescriptor';
import { MetricData, MetricDescriptor } from '../export/MetricData';
import { Maybe } from '../utils';

/** The kind of aggregator. */
Expand Down Expand Up @@ -128,14 +127,14 @@ export interface Aggregator<T> {
/**
* Returns the {@link MetricData} that this {@link Aggregator} will produce.
*
* @param descriptor the metric instrument descriptor.
* @param descriptor the metric descriptor.
* @param aggregationTemporality the temporality of the resulting {@link MetricData}
* @param accumulationByAttributes the array of attributes and accumulation pairs.
* @param endTime the end time of the metric data.
* @return the {@link MetricData} that this {@link Aggregator} will produce.
*/
toMetricData(
descriptor: InstrumentDescriptor,
descriptor: MetricDescriptor,
aggregationTemporality: AggregationTemporality,
accumulationByAttributes: AccumulationRecord<T>[],
endTime: HrTime
Expand Down
18 changes: 15 additions & 3 deletions packages/sdk-metrics/src/export/MetricData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,30 @@
* limitations under the License.
*/

import { HrTime, MetricAttributes } from '@opentelemetry/api';
import { HrTime, MetricAttributes, ValueType } from '@opentelemetry/api';
import { InstrumentationScope } from '@opentelemetry/core';
import { IResource } from '@opentelemetry/resources';
import { InstrumentDescriptor } from '../InstrumentDescriptor';
import { InstrumentType } from '../InstrumentDescriptor';
import { AggregationTemporality } from './AggregationTemporality';
import { Histogram, ExponentialHistogram } from '../aggregator/types';

export interface MetricDescriptor {
pichlermarc marked this conversation as resolved.
Show resolved Hide resolved
readonly name: string;
readonly description: string;
readonly unit: string;
/**
* @deprecated exporter should avoid depending on the type of the instrument
* as their resulting aggregator can be re-mapped with views.
*/
readonly type: InstrumentType;
readonly valueType: ValueType;
}

/**
* Basic metric data fields.
*/
interface BaseMetricData {
readonly descriptor: InstrumentDescriptor;
readonly descriptor: MetricDescriptor;
readonly aggregationTemporality: AggregationTemporality;
/**
* DataPointType of the metric instrument.
Expand Down
1 change: 1 addition & 0 deletions packages/sdk-metrics/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export {
ResourceMetrics,
ScopeMetrics,
MetricData,
MetricDescriptor,
CollectionResult,
} from './export/MetricData';

Expand Down
1 change: 1 addition & 0 deletions packages/sdk-metrics/src/state/MetricStorage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export abstract class MetricStorage {
description: description,
valueType: this._instrumentDescriptor.valueType,
unit: this._instrumentDescriptor.unit,
advice: this._instrumentDescriptor.advice,
}
);
}
Expand Down
5 changes: 4 additions & 1 deletion packages/sdk-metrics/src/state/ObservableRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,10 @@ export class ObservableRegistry {

private _observeCallbacks(observationTime: HrTime, timeoutMillis?: number) {
return this._callbacks.map(async ({ callback, instrument }) => {
const observableResult = new ObservableResultImpl(instrument._descriptor);
const observableResult = new ObservableResultImpl(
instrument._descriptor.name,
instrument._descriptor.valueType
);
let callPromise: Promise<void> = Promise.resolve(
callback(observableResult)
);
Expand Down
5 changes: 5 additions & 0 deletions packages/sdk-metrics/src/view/Aggregation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,11 @@ export class DefaultAggregation extends Aggregation {
return LAST_VALUE_AGGREGATION;
}
case InstrumentType.HISTOGRAM: {
if (instrument.advice.explicitBucketBoundaries) {
return new ExplicitBucketHistogramAggregation(
instrument.advice.explicitBucketBoundaries
);
}
pichlermarc marked this conversation as resolved.
Show resolved Hide resolved
return HISTOGRAM_AGGREGATION;
}
}
Expand Down
Loading