Skip to content

[sdk.logs] Fix the version reported by LogRecord.Logger when using ILogger integration #4635

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.

Sign up for GitHub

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

Merged
CodeBlanch merged 5 commits into from
Jul 10, 2023
Merged

[sdk.logs] Fix the version reported by LogRecord.Logger when using ILogger integration #4635

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
12f3b6f
Fix the version reported by LogRecord.Logger when using ILogger integ…
CodeBlanch Jul 5, 2023
3dc824e
Code review.
CodeBlanch Jul 5, 2023
62ed5d6
Warning fix.
CodeBlanch Jul 5, 2023
84b3f02
ParseAssemblyInformationalVersion tests.
CodeBlanch Jul 6, 2023
b806633
Merge from main.
CodeBlanch Jul 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Code review.
  • Loading branch information
@CodeBlanch
CodeBlanch committed Jul 5, 2023
commit 3dc824e511caa3cc4f4842a84aaa2a0d54fb691e
2 changes: 1 addition & 1 deletion src/OpenTelemetry/Logs/ILogger/OpenTelemetryLogger.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ private LoggerInstrumentationScope(string name, string version)
}

public static LoggerInstrumentationScope Instance { get; }
= new("OpenTelemetry", Sdk.Version);
= new("OpenTelemetry", Sdk.InformationalVersion);

public override void EmitLog(in LogRecordData data, in LogRecordAttributeList attributes)
=> throw new NotSupportedException();
Expand Down
2 changes: 1 addition & 1 deletion src/OpenTelemetry/Resources/ResourceBuilderExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public static class ResourceBuilderExtensions
{
[ResourceSemanticConventions.AttributeTelemetrySdkName] = "opentelemetry",
[ResourceSemanticConventions.AttributeTelemetrySdkLanguage] = "dotnet",
[ResourceSemanticConventions.AttributeTelemetrySdkVersion] = Sdk.Version,
[ResourceSemanticConventions.AttributeTelemetrySdkVersion] = Sdk.InformationalVersion,
});

/// <summary>
Expand Down
23 changes: 18 additions & 5 deletions src/OpenTelemetry/Sdk.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,31 @@ static Sdk()
Activity.ForceDefaultIdFormat = true;
SelfDiagnostics.EnsureInitialized();

var sdkVersion = typeof(Sdk).Assembly.GetCustomAttribute<AssemblyFileVersionAttribute>()?.Version;
Version = sdkVersion != null && System.Version.TryParse(sdkVersion, out var parsedVersion)
? parsedVersion.ToString(3)
: "1.0.0";
var assemblyInformationalVersion = typeof(Sdk).Assembly.GetCustomAttribute<AssemblyInformationalVersionAttribute>()?.InformationalVersion;
if (string.IsNullOrWhiteSpace(assemblyInformationalVersion))
{
assemblyInformationalVersion = "1.0.0";
}

/*
* InformationalVersion will be in the following format:
* {majorVersion}.{minorVersion}.{patchVersion}.{pre-release label}.{pre-release version}.{gitHeight}+{Git SHA of current commit}
* Ex: 1.5.0-alpha.1.40+807f703e1b4d9874a92bd86d9f2d4ebe5b5d52e4
* The following parts are optional: pre-release label, pre-release version, git height, Git SHA of current commit
*/

var indexOfPlusSign = assemblyInformationalVersion.IndexOf('+');
InformationalVersion = indexOfPlusSign > 0
? assemblyInformationalVersion.Substring(0, indexOfPlusSign)
: assemblyInformationalVersion;
}

/// <summary>
/// Gets a value indicating whether instrumentation is suppressed (disabled).
/// </summary>
public static bool SuppressInstrumentation => SuppressInstrumentationScope.IsSuppressed;

internal static string Version { get; }
internal static string InformationalVersion { get; }
Copy link
Contributor

@utpilla utpilla Jul 6, 2023

Choose a reason for hiding this comment

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

Suggested change
internal static string InformationalVersion { get; }
internal static readonly string InformationalVersion;

Copy link
Member Author

@CodeBlanch CodeBlanch Jul 6, 2023

Choose a reason for hiding this comment

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

I'm not opposed to this, but why? 🤣 A property with just a getter ({ get; }) is readonly for the backing generated by the compiler IIRC.

Copy link
Contributor

@utpilla utpilla Jul 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

but why?

Just because it's more obvious and intuitive when you use readonly field 😀


/// <summary>
/// Sets the Default TextMapPropagator.
Expand Down
18 changes: 1 addition & 17 deletions test/OpenTelemetry.Tests/Logs/LogRecordTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
using System.Collections;
using System.Diagnostics;
using System.Globalization;
using System.Reflection;
using Microsoft.Extensions.Logging;
using OpenTelemetry.Exporter;
using OpenTelemetry.Logs;
Expand All @@ -29,21 +28,6 @@ namespace OpenTelemetry.Logs.Tests
{
public sealed class LogRecordTest
{
private static readonly string OpenTelemetrySdkVersion;

static LogRecordTest()
{
var sdkVersion = typeof(Sdk).Assembly.GetCustomAttribute<AssemblyFileVersionAttribute>()?.Version;
if (sdkVersion != null)
{
OpenTelemetrySdkVersion = Version.Parse(sdkVersion).ToString(3);
}
else
{
OpenTelemetrySdkVersion = "0.0.0";
}
}

private enum Field
{
FormattedMessage,
Expand Down Expand Up @@ -1009,7 +993,7 @@ public void LogRecordInstrumentationScopeTest()
Assert.NotNull(logRecord);
Assert.NotNull(logRecord.Logger);
Assert.Equal("OpenTelemetry", logRecord.Logger.Name);
Assert.Equal(OpenTelemetrySdkVersion, logRecord.Logger.Version);
Assert.Equal(Sdk.InformationalVersion, logRecord.Logger.Version);
}

private static ILoggerFactory InitializeLoggerFactory(out List<LogRecord> exportedItems, Action<OpenTelemetryLoggerOptions> configure = null)
Expand Down