-
Notifications
You must be signed in to change notification settings - Fork 833
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
Adds Metrics API #105
Adds Metrics API #105
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
/** | ||
* Copyright 2019, 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. | ||
*/ | ||
|
||
export interface CounterTimeseries { | ||
// Adds the given value to the current value. Values cannot be negative. | ||
add(value: number): void; | ||
|
||
// Sets the given value. Value must be larger than the current recorded value. | ||
set(value: number): void; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should these be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah probably. I'm also curious where the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would think that fundamentally a "counter" represents counting something, which intuitively can't be negative. If a value can be negative, then to me it is not really a count of something tangible but rather represents something subtly different. For example, what does it mean that a server served -20 requests? On the other hand, I could see something like "available CPU-seconds" being negative if the process is in debt to its scheduler. But that metric type I would say is a gauge and not a counter. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Side note, based on the rfcs it looks like counters only have There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, interesting - these are squishy concepts in some ways. Within the Stackdriver Monitoring product, counters (called "cumulative" in our API) have the interesting difference from gauges, in that you can take the rate of a cumulative but can't for a gauge. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
/** | ||
* Copyright 2019, 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. | ||
*/ | ||
|
||
export interface GaugeTimeseries { | ||
// Adds the given value to the current value. Values can be negative. | ||
add(value: number): void; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the purpose of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not entirely sure, it looks to be pretty close to what census had. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OpenCensus has There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah cool was looking at this, didn't see the impl for cumulative. There's currently an RFC from @jmacd to change the verbiage for metrics, along with removing meter.record https://github.com/open-telemetry/rfcs/pull/4/files#diff-a4b53eafb1f61334abc351feb3ecc5b3R20 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would agree that |
||
// Sets the given value. Values can be negative. | ||
set(value: number): void; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
/** | ||
* Copyright 2019, 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. | ||
*/ | ||
|
||
export enum MeasureType { | ||
DOUBLE = 0, | ||
LONG = 1, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we don't have a plan yet for the |
||
} | ||
|
||
export interface MeasureOptions { | ||
// Description of the Measure. | ||
description?: string; | ||
|
||
// Unit of the Measure. | ||
unit?: string; | ||
|
||
// Type of the Measure. Default type is DOUBLE. | ||
type?: MeasureType; | ||
} | ||
|
||
export interface Measure { | ||
// Creates a measurement with the supplied value. | ||
createMeasurement(value: number): Measurement; | ||
} | ||
|
||
// Measurement describes an individual measurement | ||
export interface Measurement {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
/** | ||
* Copyright 2019, 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 { SpanContext } from '../trace/span_context'; | ||
import { DistributedContext } from '../distributed_context/DistributedContext'; | ||
import { Measure, MeasureOptions, Measurement } from './measure'; | ||
import { Metric, MetricOptions } from './metric'; | ||
import { CounterTimeseries } from './counter'; | ||
import { GaugeTimeseries } from './gauge'; | ||
|
||
export interface RecordOptions { | ||
// spanContext represents a measurement exemplar in the form of a SpanContext. | ||
spanContext?: SpanContext; | ||
// distributedContext overrides the current context and adds dimensions | ||
// to the measurements. | ||
distributedContext?: DistributedContext; | ||
} | ||
|
||
export interface Meter { | ||
// Creates and returns a new @link{Measure}. | ||
createMeasure(name: string, options?: MeasureOptions): Measure; | ||
|
||
// Creates a new counter metric. | ||
createCounter( | ||
name: string, | ||
options?: MetricOptions | ||
): Metric<CounterTimeseries>; | ||
|
||
// TODO: Measurements can have a long or double type. However, it looks like | ||
// the metric timeseries API (according to spec) accepts values instead of | ||
// Measurements, meaning that if you accept a `number`, the type gets lost. | ||
// Both java and csharp have gone down the route of having two gauge interfaces, | ||
// GaugeDoubleTimeseries and GaugeLongTimeseries, with param for that type. It'd | ||
// be cool to only have a single interface, but maybe having two is necessary? | ||
// Maybe include the type as a metrics option? Probs a good gh issue, the same goes for Measure types. | ||
|
||
// Creates a new gauge metric. | ||
createGauge(name: string, options?: MetricOptions): Metric<GaugeTimeseries>; | ||
|
||
// Record a set of raw measurements. | ||
record(measurements: Measurement[], options?: RecordOptions): void; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
/** | ||
* Copyright 2019, 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 { Attributes } from '../trace/attributes'; | ||
import { Resource } from '../resources/Resource'; | ||
|
||
export interface MetricOptions { | ||
// Description of the Metric. | ||
description?: string; | ||
|
||
// Unit of the Metric values. | ||
unit?: string; | ||
|
||
// List of attribute keys with dynamic values. Order of list is important | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As an aside, I feel that if we're passing Key:Value objects to the metrics / stats APIs, then we should be able to relax this statement about the order being important. IOW I interpret the |
||
// as the same order must be used when supplying values for these attributes. | ||
dynamicAttributes?: string[]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be a Map instead? Otherwise it will be difficult to update a value in the list. |
||
|
||
// List of attributes with constant values. | ||
constantAttributes?: Attributes; | ||
|
||
// Resource the metric is associated with. | ||
resource?: Resource; | ||
|
||
// Name of the component that reports the metric. | ||
component?: string; | ||
} | ||
|
||
// Metric represents a base class for different types of metric preaggregations. | ||
export interface Metric<T> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are there any bounds on what type There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sadly it's not really spelled out in the specification but there's a somewhat hidden type called Right now Counter and Gauge have the same interface in the specification (although maybe they shouldn't). I'm curious to see what the histogram and summary types are going to look like. edit: It's these interfaces here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I realize this is already merged, but getting caught up here. When I see a templated type like But in this case it's really more like |
||
// Creates a timeseries if the specified attribute values | ||
// are not associated with an existing timeseries, otherwise returns the | ||
// existing timeseries. | ||
// Order and number of attribute values MUST match the order and number of | ||
// dynanic attribute keys when the Metric was created. | ||
getOrCreateTimeseries(values: unknown[]): T; | ||
|
||
// Returns a timeseries with all attribute values not set. | ||
getDefaultTimeseries(): T; | ||
|
||
// Removes an existing timeseries. Order and number of attribute values MUST | ||
// match the order and number of dynamic attribute keys when the Metric was | ||
// created. | ||
removesTimeseries(values: unknown[]): void; | ||
|
||
// Clears all timeseries from the Metric. | ||
clear(): void; | ||
|
||
// todo: what should the callback signature be? | ||
setCallback(fn: () => void): void; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a topic for the SIG, but would it be worth putting a comment explaining that
bigint
is not currently supported but may be added in a future release?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's a good topic, added to the sig agenda.