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

Instrument<T>.RecordMeasurement does not record if given empty TagList on .NET Framework #97948

Closed
KalleOlaviNiemitalo opened this issue Feb 4, 2024 · 3 comments

Comments

@KalleOlaviNiemitalo
Copy link

Description

On .NET Framework, if the protected void RecordMeasurement(T measurement, in TagList tagList) method of System.Diagnostics.Metrics.Instrument<T> is given an empty TagList tagList, then the method does not record T measurement.

Reproduction Steps

NoTags.csproj:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFrameworks>net48;net8.0</TargetFrameworks>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="System.Diagnostics.DiagnosticSource" Version="8.0.0" />
  </ItemGroup>

</Project>

Program.cs:

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Diagnostics.Metrics;

namespace NoTags
{
    internal static class Program
    {
        private static void Main()
        {
            using (MeterListener meterListener = new MeterListener())
            using (Meter meter = new Meter("demo"))
            {
                Counter<int> first = meter.CreateCounter<int>("first");
                Counter<int> second = meter.CreateCounter<int>("second");
                Counter<int> third = meter.CreateCounter<int>("third");
                Counter<int> fourth = meter.CreateCounter<int>("fourth");

                meterListener.SetMeasurementEventCallback<int>(ShowMeasurement);
                meterListener.EnableMeasurementEvents(first);
                meterListener.EnableMeasurementEvents(second);
                meterListener.EnableMeasurementEvents(third);
                meterListener.EnableMeasurementEvents(fourth);

                first.Add(1); // OK
                second.Add(1, new TagList()); // lost
                third.Add(1, Array.Empty<KeyValuePair<string, object>>()); // OK
                fourth.Add(1, new TagList(Array.Empty<KeyValuePair<string, object>>())); // lost
            }
        }

        private static void ShowMeasurement<T>(
            Instrument instrument,
            T measurement,
            ReadOnlySpan<KeyValuePair<string, object>> tags,
            object state)
        {
            Console.WriteLine($"{instrument.Name} = {measurement}");
        }
    }
}

Run this program on each target framework.

Expected behavior

Each Add call should trigger the measurement callback of the MeterListener. The behaviour should be the same on .NET Framework and .NET.

$ dotnet run --framework=net8.0
first = 1
second = 1
third = 1
fourth = 1

$ dotnet run --framework=net48
first = 1
second = 1
third = 1
fourth = 1

Actual behavior

On .NET Framework only, the Add calls that pass in an empty TagList do not trigger the measurement callback of the MeterListener. However, the Add calls that do not involve a TagList parameter still trigger the callback, even though those calls don't have any tags either.

$ dotnet run --framework=net8.0
first = 1
second = 1
third = 1
fourth = 1

$ dotnet run --framework=net48
first = 1
third = 1

Regression?

Not a regression: version 6.0.0 of the System.Diagnostics.DiagnosticSource package already has this bug on .NET Framework, and earlier versions do not define the TagList type.

Known Workarounds

If you don't have tags, then don't use the overload that has a TagList parameter.

Configuration

System.Diagnostics.DiagnosticSource 8.0.0 on .NET Framework 4.8 on Windows 10 version 22H2 x64.
Built using .NET SDK 8.0.101.
The bug affects only software built targeting .NET Framework.

Other information

This return statement and its comment are not right:

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Feb 4, 2024
@KalleOlaviNiemitalo KalleOlaviNiemitalo changed the title Instrument.RecordMeasurement does not record if given empty TagList on .NET Framework Instrument<T>.RecordMeasurement does not record if given empty TagList on .NET Framework Feb 4, 2024
@tarekgh
Copy link
Member

tarekgh commented Jun 20, 2024

@KalleOlaviNiemitalo thanks for reporting the issue. Are you interested in submitting a PR for the fix?

@tarekgh tarekgh removed the untriaged New issue has not been triaged by the area owner label Jun 20, 2024
@tarekgh tarekgh added this to the 9.0.0 milestone Jun 20, 2024
@tarekgh tarekgh self-assigned this Jun 20, 2024
@KalleOlaviNiemitalo
Copy link
Author

No I'm not.

@tarekgh
Copy link
Member

tarekgh commented Jun 20, 2024

No problem. Thanks!

@tarekgh tarekgh closed this as completed Jun 21, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jul 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants