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

[Feature Request]: Add synchronous gauge instrument #92625

Closed
noahfalk opened this issue Sep 26, 2023 · 10 comments · Fixed by #103543
Closed

[Feature Request]: Add synchronous gauge instrument #92625

noahfalk opened this issue Sep 26, 2023 · 10 comments · Fixed by #103543
Labels
api-approved API was approved in API review, it can be implemented area-System.Diagnostics.Metric blocking Marks issues that we want to fast track in order to unblock other important work feature-request in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@noahfalk
Copy link
Member

noahfalk commented Sep 26, 2023

Edited by @tarekgh

OpenTelemetry has introduced the Gauge instrument, designed to record non-additive values when changes occur. For example, it can measure the background noise level, where summing the values from multiple rooms would be nonsensical.

Proposal

namespace System.Diagnostics.Metrics;

public class Meter : IDisposable
{
	public Gauge<T> CreateGauge<T>(string name) where T : struct {}

	public Gauge<T> CreateGauge<T>(string name, string? unit = null, string? description = null, IEnumerable<KeyValuePair<string, object?>>? tags = null) where T : struct {}
}

public sealed class Gauge<T> : Instrument<T> where T : struct
{
	public void Record(T value) {}

	public void Record(T value, KeyValuePair<string, object?> tag) { }

	public void Record(T value, KeyValuePair<string, object?> tag1, KeyValuePair<string, object?> tag2) { }

	public void Record(T value, KeyValuePair<string, object?> tag1, KeyValuePair<string, object?> tag2, KeyValuePair<string, object?> tag3) { }

	public void Record(T value, params ReadOnlySpan<KeyValuePair<string, object?>> tags) { }

	public void Record(T value, params KeyValuePair<string, object?>[] tags) { }

	public void Record(T value, in TagList tagList) { }
	
	//
	// Instrument<T> properties
	//
	
	public Meter Meter { get; }
        public string Name { get; }
        public string? Description { get; }
        public string? Unit { get; }
        public IEnumerable<KeyValuePair<string, object?>>? Tags { get; }
        public bool Enabled { get; }
        public virtual bool IsObservable { get; }
}

Usage Example

    Meter meter = new Meter("MeasurmentLibrary.Sound");
 
    Gauge<int> gauge= meter.CreateGauge<int>(name: "NoiseLevel", unit: "dB", description: "Background Noise Level"); // dB is Decibel, the sound intensity level unit
    
    gauge.Record(10, new TagList { "Room1", "dB"});

Original Description

OpenTelemetry has now defined a synchronous gauge. If the OTel maintainers agree the spec is sufficiently stable we should implement it in System.Diagnostics.Metrics. Feel free to replace this stub with a more complete description of the new API surface.

In addition to adding the API, we also need to add support for it to the MetricsEventSource.

OTel issue: open-telemetry/opentelemetry-dotnet#4805

cc @tarekgh

@noahfalk noahfalk added this to the 9.0.0 milestone Sep 26, 2023
@noahfalk noahfalk added enhancement Product code improvement that does NOT require public API changes/additions feature-request and removed enhancement Product code improvement that does NOT require public API changes/additions labels Sep 26, 2023
@lsoft
Copy link

lsoft commented Apr 11, 2024

Does any workaround exists now?

I've written

public sealed class SingleItemGauge<T>
    where T : struct
{
    private readonly ObservableGauge<T> _gauge;

    private bool _valueSent = false;

    public bool ValueSent => _valueSent;

    public SingleItemGauge(
        Meter parentMeter,
        string counterName,
        string counterDescription,
        string counterUnit,
        T counterValue,
        KeyValuePair<string, object?>[] counterTags
        )
    {
        _gauge =
            parentMeter.CreateObservableGauge<T>(
                name: counterName,
                () =>
                {
                    if (_valueSent)
                    {
                        return new Measurement<T>[0];
                    }

                    _valueSent = true;

                    return
                        new[]
                        {
                            new Measurement<T>(
                                value: counterValue,
                                tags: counterTags
                                )
                        };
                },
                unit: counterUnit,
                description: counterDescription
                );
    }
}

but I'm not sure if return new Measurement<T>[0]; means that no more data will be send to otel.

@KalleOlaviNiemitalo
Copy link

@lsoft, your _valueSent workaround looks like it doesn't support multiple MeterListener instances. Calling MeterListener.RecordObservableInstruments() on any of them causes _valueSent to be set, and then the measurement callbacks of the other MeterListener instances won't be called.

@lsoft
Copy link

lsoft commented Apr 11, 2024

@KalleOlaviNiemitalo I'm a newbie in otel\prometheus area, so does I understand correctly that MeterListener.RecordObservableInstruments obtain "my value" but does not send it? if so, does any way to workaround this exists?

anyway, my workaround is just a workaround in absense of "NonObservableGauge" and I'm using it in the application where only one Meter and MeterProvider exists and non of MeterListener declared explicitly...

I've only been learning OTEL+Prometheus for a few days, so I understand a little, but looks like dotnet OTEL is in early stage of its life. For example, prometheus exemplars is not supported, sync Gauge is not supported... etc... Will wait for its mature :)

@KalleOlaviNiemitalo
Copy link

@lsoft my hunch is that the per-instrument mutable state will become a problem if the application uses both the OpenTelemetry .NET library and some other library that likewise creates a MeterListener instance and listens to the same instrument. But it may work ok if you are only using one library for the telemetry and that internally distributes the data to multiple exporters.

@xakep139
Copy link
Member

xakep139 commented Jun 3, 2024

@lsoft you can also check JS polyfill shared in open-telemetry/opentelemetry-specification#2318 (comment) and adopt it to C#,
The solution there does support updating the value to be reported though.

@tarekgh
Copy link
Member

tarekgh commented Jun 3, 2024

@cijothomas @CodeBlanch @noahfalk @reyang could you please let me know if you have any feedback regarding the proposal #92625 (comment)?

@cijothomas
Copy link
Contributor

@cijothomas @CodeBlanch @noahfalk @reyang could you please let me know if you have any feedback regarding the proposal #92625 (comment)?

LGTM.

@CodeBlanch
Copy link
Contributor

LGTM

1 similar comment
@noahfalk
Copy link
Member Author

noahfalk commented Jun 3, 2024

LGTM

@tarekgh tarekgh added api-ready-for-review API is ready for review, it is NOT ready for implementation blocking Marks issues that we want to fast track in order to unblock other important work labels Jun 3, 2024
@terrajobst
Copy link
Member

terrajobst commented Jun 11, 2024

Video

Looks good as proposed.

namespace System.Diagnostics.Metrics;

public partial class Meter : IDisposable
{
	public static Gauge<T> CreateGauge<T>(string name) where T : struct;
	public static Gauge<T> CreateGauge<T>(string name, string? unit = null, string? description = null, IEnumerable<KeyValuePair<string, object?>>? tags = null) where T : struct;
}

public sealed class Gauge<T> : Instrument<T> where T : struct
{
	public void Record(T value);
	public void Record(T value, KeyValuePair<string, object?> tag);
	public void Record(T value, KeyValuePair<string, object?> tag1, KeyValuePair<string, object?> tag2);
	public void Record(T value, KeyValuePair<string, object?> tag1, KeyValuePair<string, object?> tag2, KeyValuePair<string, object?> tag3);
	public void Record(T value, params ReadOnlySpan<KeyValuePair<string, object?>> tags);
	public void Record(T value, params KeyValuePair<string, object?>[] tags);
	public void Record(T value, in TagList tagList);
}

@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Jun 11, 2024
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Jun 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-approved API was approved in API review, it can be implemented area-System.Diagnostics.Metric blocking Marks issues that we want to fast track in order to unblock other important work feature-request in-pr There is an active PR which will close this issue when it is merged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants