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

[prototype] feat(api): add wrapMeter() for experimental metrics API features #4622

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions api/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ All notable changes to this project will be documented in this file.

### :rocket: (Enhancement)

* feat(metrics): added synchronous gauge [#4528](https://github.com/open-telemetry/opentelemetry-js/pull/4528) @clintonb
* feat: support node 22 [#4666](https://github.com/open-telemetry/opentelemetry-js/pull/4666) @dyladan

### :bug: (Bug Fix)
Expand Down
10 changes: 10 additions & 0 deletions api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,16 @@
"default": "./build/src/experimental/index.js"
}
},
"typesVersions": {
"*": {
"*": [
"./build/src/index.d.ts"
],
"experimental": [
"./build/src/experimental/index.d.ts"
]
}
},
"repository": "open-telemetry/opentelemetry-js",
"scripts": {
"clean": "tsc --build --clean tsconfig.json tsconfig.esm.json tsconfig.esnext.json",
Expand Down
9 changes: 9 additions & 0 deletions api/src/experimental/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,14 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/

// Trace
export { wrapTracer, SugaredTracer } from './trace/SugaredTracer';
export { SugaredSpanOptions } from './trace/SugaredOptions';

// Metrics
export {
wrapMeter,
Gauge,
IExperimentalMeter,
} from './metrics/ExperimentalMeter';
129 changes: 129 additions & 0 deletions api/src/experimental/metrics/ExperimentalMeter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
/*
* Copyright The OpenTelemetry Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { Meter } from '../../metrics/Meter';
import { MetricAttributes, MetricOptions } from '../../metrics/Metric';
import { Context } from '../../context/types';
import { NOOP_GAUGE_METRIC } from './Gauge';

/**
* @experimental
*
* Records only the last value that is added to it, discards any others.
*/
export interface Gauge<
AttributesTypes extends MetricAttributes = MetricAttributes,
> {
/**
* @experimental
* Records a measurement.
*/
record(value: number, attributes?: AttributesTypes, context?: Context): void;
}

/**
* Steps for adding new experimental Meter API features:
* - implement the specification in the `@opentelemetry/sdk-metrics` package, do NOT use any new types
* - SDKs may peer-depend on old API versions where these types are not yet available
* - we MUST duplicate any new types across API and SDK unless we drop support for older API versions
* - dropping support for old API versions MUST be done in SDK Major versions
* - failure to do this will result in failing builds for users that update the SDK but not the API
* - add the new functionality to the {@link IExperimentalMeter} interface
* - implement the functionality in the {@link ExperimentalMeter}
* - if the underlying {@link Meter} used as an {@link IExperimentalMeter} throws an error, return a No-Op implementation.
*
* Users may now use {@link wrapMeter} to get access to experimental SDK functionality via the API
*
* To stabilize a feature:
* - move the function interface from `IExperimentalMeter` to `Meter`
* - replace the implemented method with a property of the same type as the one from `Meter`, move any auxiliary types to stable
* - In ExperimentalMeter
* - remove the implementation that falls back to No-Op on errors.
* - assign the function that was moved to `Meter` to the property defined in the last step
* - Implement the no-op case in {@link NoopMeter}
* - ensure any `@experimental` annotations are removed
*/

/**
* @experimental
*
* Meter that offers experimental functionality IF that functionality is implemented by the SDK.
* MAY return a no-op otherwise. Stable features continue to work as expected.
*/
export interface IExperimentalMeter extends Meter {
/**
* @experimental Will be added to {@link Meter} in a future version when the specification is marked stable
*
* Creates and returns a new `Gauge`.
* @param name the name of the metric.
* @param [options] the metric options.
*/
createGauge<AttributesTypes extends MetricAttributes = MetricAttributes>(
name: string,
options?: MetricOptions
): Gauge<AttributesTypes>;
}

class ExperimentalMeter implements IExperimentalMeter {
private _meter: Meter;
constructor(meter: Meter) {
this._meter = meter;
this.addBatchObservableCallback =
meter.addBatchObservableCallback.bind(meter);
this.createCounter = meter.createCounter.bind(meter);
this.createObservableGauge = meter.createObservableGauge.bind(meter);
this.createHistogram = meter.createHistogram.bind(meter);
this.createObservableCounter = meter.createObservableCounter.bind(meter);
this.createObservableUpDownCounter =
meter.createObservableUpDownCounter.bind(meter);
this.createUpDownCounter = meter.createUpDownCounter.bind(meter);
this.removeBatchObservableCallback =
meter.removeBatchObservableCallback.bind(meter);
}
addBatchObservableCallback: Meter['addBatchObservableCallback'];
createCounter: Meter['createCounter'];
createObservableGauge: Meter['createObservableGauge'];
createHistogram: Meter['createHistogram'];
createObservableCounter: Meter['createObservableCounter'];
createUpDownCounter: Meter['createUpDownCounter'];
createObservableUpDownCounter: Meter['createObservableUpDownCounter'];
removeBatchObservableCallback: Meter['removeBatchObservableCallback'];
Comment on lines +81 to +102
Copy link
Member

Choose a reason for hiding this comment

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

This seems like a lot of duplication and large strings. I doubt this is very friendly to minification. Isn't there a way to keep the meter mostly the same and put new methods on it without a whole new class?

Copy link
Member Author

Choose a reason for hiding this comment

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

The strings actually don't end up in the final bundle as they're only used for the types. The rest of the code, however is not the most efficient in terms of bundle-size, you're right.

Since the API we were trying to add here was marked as stable, I'll close this PR and re-focus my efforts on #4669


/**
* Creates and returns a new `Gauge`.
* @param name the name of the metric.
* @param [options] the metric options.
*/
createGauge(name: string, options?: MetricOptions): Gauge {
try {
return (this._meter as ExperimentalMeter).createGauge(name, options);
} catch (e) {
return NOOP_GAUGE_METRIC;
}
}
}

/**
* @experimental
*
* Wraps {@link Meter} so that it offers experimental functionality IF that functionality is implemented by the
* registered SDK. MAY return a no-op instrument if the functionality is not implemented by the SDK.
* Stable features continue to work as expected.
*
* @param meter
*/
export function wrapMeter(meter: Meter): IExperimentalMeter {
return new ExperimentalMeter(meter);
}
31 changes: 31 additions & 0 deletions api/src/experimental/metrics/Gauge.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* Copyright The OpenTelemetry Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { Gauge } from './ExperimentalMeter';
import { MetricAttributes } from '../../metrics/Metric';
import { NoopMetric } from '../../metrics/NoopMeter';

/**
* @experimental
*/
export class NoopGaugeMetric extends NoopMetric implements Gauge {
record(_value: number, _attributes: MetricAttributes): void {}
}

/**
* @experimental
*/
export const NOOP_GAUGE_METRIC = new NoopGaugeMetric();
4 changes: 2 additions & 2 deletions api/src/metrics/NoopMeter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@ import {
BatchObservableCallback,
Counter,
Histogram,
MetricAttributes,
MetricOptions,
Observable,
ObservableCallback,
ObservableCounter,
ObservableGauge,
ObservableUpDownCounter,
UpDownCounter,
MetricAttributes,
Observable,
} from './Metric';

/**
Expand Down
13 changes: 13 additions & 0 deletions api/test/common/noop-implementations/noop-meter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import {
createNoopMeter,
} from '../../../src/metrics/NoopMeter';
import { NoopMeterProvider } from '../../../src/metrics/NoopMeterProvider';
import { wrapMeter } from '../../../src/experimental';
import { NOOP_GAUGE_METRIC } from '../../../src/experimental/metrics/Gauge';

const attributes = {};
const options = {
Expand Down Expand Up @@ -116,6 +118,17 @@ describe('NoopMeter', () => {
);
});

it('gauge should not crash', () => {
const meter = new NoopMeterProvider().getMeter('test-noop');
const observableGauge = wrapMeter(meter).createGauge('some-name');

// ensure the correct noop const is returned
assert.strictEqual(observableGauge, NOOP_GAUGE_METRIC);

const gaugeWithOptions = wrapMeter(meter).createGauge('some-name', options);
assert.strictEqual(gaugeWithOptions, NOOP_GAUGE_METRIC);
});

it('observable up down counter should not crash', () => {
const meter = new NoopMeterProvider().getMeter('test-noop');
const observableUpDownCounter =
Expand Down
48 changes: 42 additions & 6 deletions packages/sdk-metrics/test/Instruments.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,27 +19,28 @@ import * as sinon from 'sinon';
import { InstrumentationScope } from '@opentelemetry/core';
import { Resource } from '@opentelemetry/resources';
import {
InstrumentType,
MeterProvider,
MetricReader,
DataPoint,
DataPointType,
Histogram,
InstrumentType,
MeterProvider,
MetricDescriptor,
MetricReader,
} from '../src';
import {
TestDeltaMetricReader,
TestMetricReader,
} from './export/TestMetricReader';
import {
assertMetricData,
assertDataPoint,
commonValues,
assertMetricData,
commonAttributes,
defaultResource,
commonValues,
defaultInstrumentationScope,
defaultResource,
} from './util';
import { ObservableResult, ValueType } from '@opentelemetry/api';
import { wrapMeter } from '@opentelemetry/api/experimental';

describe('Instruments', () => {
describe('Counter', () => {
Expand Down Expand Up @@ -764,6 +765,41 @@ describe('Instruments', () => {
});
});
});

describe('Gauge', () => {
it('should record common values and attributes without exceptions', async () => {
const { meter } = setup();
const gauge = wrapMeter(meter).createGauge('test');

for (const values of commonValues) {
for (const attributes of commonAttributes) {
gauge.record(values, attributes);
}
}
});

it('should record values', async () => {
const { meter, cumulativeReader } = setup();
const gauge = wrapMeter(meter).createGauge('test');

gauge.record(1, { foo: 'bar' });
gauge.record(-1);

await validateExport(cumulativeReader, {
dataPointType: DataPointType.GAUGE,
dataPoints: [
{
attributes: { foo: 'bar' },
value: 1,
},
{
attributes: {},
value: -1,
},
],
});
});
});
});

function setup() {
Expand Down