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

Detect service.version from Assembly Version? #5703

Open
Aaronontheweb opened this issue Jun 19, 2024 · 2 comments
Open

Detect service.version from Assembly Version? #5703

Aaronontheweb opened this issue Jun 19, 2024 · 2 comments
Labels
enhancement New feature or request pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package

Comments

@Aaronontheweb
Copy link
Contributor

Package

OpenTelemetry

Is your feature request related to a problem?

I implemented this already in some of my own projects, was mostly interested in knowing if the project would accept it as a contribution if I cleaned it up and made a PR.

I usually use the AssemblyVersion as my service.version value in OTEL since the former is always synced with our Docker image tags in our build process. Therefore, I wrote an IResourceDetector that will just grab this data and use it throughout all of our OTEL-enabled projects:

public sealed class AssemblyVersionDetector : IResourceDetector
{
    public Resource Detect()
    {
        // care about the foreground assembly used in `dotnet run` / equivalent
        var version = Assembly.GetExecutingAssembly().GetName().Version?.ToString();

        IEnumerable<KeyValuePair<string, object>> attributes = Array.Empty<KeyValuePair<string, object>>();

        if (version != null)
        {
            attributes = new[]
            {
                new KeyValuePair<string, object>("service.version", version)
            };
        }
        
        return new Resource(attributes);
    }
}

Totally fine renaming it / cleaning it up to use resource dictionaries et al. But would it make sense to have this available as a built-in IResourceDetector in the core library? Even though it technically uses System.Reflection I think it should still be pass AOT-compatibility checks since it's not doing any dynamic-linking.

What is the expected behavior?

Populates the service.version with the appropriate value derived from the executing assembly's information.

You can see an example of what this looks like using Seq's OTEL log aggregation w/ data from one of our applications (0.2.4.0 is the correct value).

image

Which alternative solutions or features have you considered?

Just using my own middleware to do this

Additional context

No response

@Aaronontheweb Aaronontheweb added the enhancement New feature or request label Jun 19, 2024
@github-actions github-actions bot added the pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package label Jun 19, 2024
@CodeBlanch
Copy link
Member

@Aaronontheweb

Here are my thoughts...

  • You could definitely add this as-is over in https://github.com/open-telemetry/opentelemetry-dotnet-contrib. If you want to just do that and stop reading, fine by me.

  • I think this is useful enough it could be part of the main SDK. I don't speak for all approvers/maintainers, but I would be supportive of something like...

     public static ResourceBuilder AddServiceVersionDetector(
        this ResourceBuilder resourceBuilder,
        Assembly targetAssembly,
        ServiceResourceVersionDetectionType detectionType)
     {
        // ...
     }
    
    public enum ServiceResourceVersionDetectionType
    {
       AssemblyVersion,
       AssemblyFileVersion,
       AssemblyInformationVersion
    }

    I added the ability to look at AssemblyFileVersionAttribute & AssemblyInformationalVersionAttribute as well via the enum. For a project like OTel, AssemblyVersion is always 1.0.0.0 and AssemblyFileVersion \ AssemblyInformationalVersion are the more useful things. This actually follows the guidance, but I know a lot of users don't. Basically I think we should be able to look different spots and perhaps add more in the future?

@Kielek
Copy link
Contributor

Kielek commented Jun 21, 2024

@CodeBlanch, I would vote for your extended proposal, but in the contrib repository. I do not think that it is defined in spec/semantic convention to put it as a part of SDK.

If we speaking about detection, from particular assemblies, we are doing something similar for scope version. Keep in mind that part of the versioning system extends versions by suffixes (eg. GH commits). Ref: https://github.com/open-telemetry/opentelemetry-dotnet-contrib/blob/9114d0b183def59bc0cd747bdee024aae7d35fbc/src/Shared/AssemblyVersionExtensions.cs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package
Projects
None yet
Development

No branches or pull requests

3 participants