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.

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

OpenTelemetryLoggerProvider is now unaffected by changes to OpenTelemetryLoggerOptions after the LoggerFactory is built. #3055

Merged
merged 37 commits into from
Mar 30, 2022
Merged

OpenTelemetryLoggerProvider is now unaffected by changes to OpenTelemetryLoggerOptions after the LoggerFactory is built. #3055

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
f944a1c
Merge pull request #1 from open-telemetry/main
TimothyMothra Jan 28, 2022
ba726c5
Merge branch 'open-telemetry:main' into main
TimothyMothra Feb 10, 2022
68d1977
Merge branch 'open-telemetry:main' into main
TimothyMothra Feb 15, 2022
9dcb839
poc for fix to InMemoryExporter
TimothyMothra Feb 16, 2022
34d15ec
Revert "poc for fix to InMemoryExporter"
TimothyMothra Feb 22, 2022
436f0dc
Merge branch 'open-telemetry:main' into main
TimothyMothra Feb 22, 2022
1c7390c
Merge branch 'open-telemetry:main' into main
TimothyMothra Mar 4, 2022
9d2453f
Merge branch 'open-telemetry:main' into main
TimothyMothra Mar 14, 2022
752f6fd
includescopes
TimothyMothra Mar 14, 2022
4e9fe78
Merge branch 'open-telemetry:main' into main
TimothyMothra Mar 15, 2022
e3779ad
prevent LoggerOptions from being modified after init.
TimothyMothra Mar 15, 2022
34bb684
Merge branch 'main' into 2902_fix_options
TimothyMothra Mar 15, 2022
797f991
REMOVE
TimothyMothra Mar 15, 2022
a5a8d93
Merge branch '2902_fix_options' of https://github.com/TimothyMothra/o…
TimothyMothra Mar 15, 2022
4a29ff1
copy the values out of the Options instance
TimothyMothra Mar 16, 2022
c6f2133
remove
TimothyMothra Mar 16, 2022
9a9b7c0
new test to verify options cannot be changed
TimothyMothra Mar 17, 2022
0362a97
Merge branch 'main' into 2902_fix_options
TimothyMothra Mar 17, 2022
b3ab1f9
changelog
TimothyMothra Mar 18, 2022
fe9d508
Merge branch '2902_fix_options' of https://github.com/TimothyMothra/o…
TimothyMothra Mar 18, 2022
5e68aff
Merge branch 'main' into 2902_fix_options
TimothyMothra Mar 18, 2022
dbb9645
update changelog
TimothyMothra Mar 18, 2022
73bf126
merge
TimothyMothra Mar 18, 2022
c659f74
Update src/OpenTelemetry/CHANGELOG.md
TimothyMothra Mar 18, 2022
b1b4f7b
Merge branch 'main' into 2902_fix_options
TimothyMothra Mar 22, 2022
5e71c7c
Merge branch 'main' into 2902_fix_options
TimothyMothra Mar 25, 2022
767beaf
update
TimothyMothra Mar 28, 2022
468e96a
Merge branch '2902_fix_options' of https://github.com/TimothyMothra/o…
TimothyMothra Mar 28, 2022
baea732
Merge branch 'main' into 2902_fix_options
TimothyMothra Mar 28, 2022
6cf5620
Merge branch 'main' into 2902_fix_options
TimothyMothra Mar 29, 2022
5aef460
addressing pr comments.
TimothyMothra Mar 29, 2022
b45f92b
Merge branch '2902_fix_options' of https://github.com/TimothyMothra/o…
TimothyMothra Mar 29, 2022
99f6b9e
remove CreateLogger from InitializeLoggerFactory
TimothyMothra Mar 29, 2022
8cb96b8
Merge branch 'main' into 2902_fix_options
cijothomas Mar 29, 2022
d7ba0a5
Merge branch 'main' into 2902_fix_options
TimothyMothra Mar 29, 2022
fbf6ac5
Update CHANGELOG.md
TimothyMothra Mar 29, 2022
4b760d1
Merge branch 'main' into 2902_fix_options
cijothomas Mar 29, 2022
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
49 changes: 25 additions & 24 deletions test/OpenTelemetry.Tests/Logs/LogRecordTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public sealed class LogRecordTest
[Fact]
public void CheckCateogryNameForLog()
{
InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: null);
using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: null);

logger.LogInformation("Log");
var categoryName = exportedItems[0].CategoryName;
Expand All @@ -52,7 +52,7 @@ public void CheckCateogryNameForLog()
[InlineData(LogLevel.Critical)]
public void CheckLogLevel(LogLevel logLevel)
{
InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: null);
using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: null);

var message = $"Log {logLevel}";
logger.Log(logLevel, message);
Expand All @@ -64,7 +64,7 @@ public void CheckLogLevel(LogLevel logLevel)
[Fact]
public void CheckStateForUnstructuredLog()
{
InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: null);
using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: null);

var message = "Hello, World!";
logger.LogInformation(message);
Expand All @@ -79,7 +79,7 @@ public void CheckStateForUnstructuredLog()
[Fact]
public void CheckStateForUnstructuredLogWithStringInterpolation()
{
InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: null);
using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: null);

var message = $"Hello from potato {0.99}.";
logger.LogInformation(message);
Expand All @@ -94,7 +94,7 @@ public void CheckStateForUnstructuredLogWithStringInterpolation()
[Fact]
public void CheckStateForStructuredLogWithTemplate()
{
InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: null);
using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: null);

var message = "Hello from {name} {price}.";
logger.LogInformation(message, "tomato", 2.99);
Expand All @@ -121,7 +121,7 @@ public void CheckStateForStructuredLogWithTemplate()
[Fact]
public void CheckStateForStructuredLogWithStrongType()
{
InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: null);
using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: null);

var food = new Food { Name = "artichoke", Price = 3.99 };
logger.LogInformation("{food}", food);
Expand All @@ -147,7 +147,7 @@ public void CheckStateForStructuredLogWithStrongType()
[Fact]
public void CheckStateForStructuredLogWithAnonymousType()
{
InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: null);
using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: null);

var anonymousType = new { Name = "pumpkin", Price = 5.99 };
logger.LogInformation("{food}", anonymousType);
Expand All @@ -173,7 +173,7 @@ public void CheckStateForStructuredLogWithAnonymousType()
[Fact]
public void CheckStateForStrucutredLogWithGeneralType()
{
InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: null);
using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: null);

var food = new Dictionary<string, object>
{
Expand Down Expand Up @@ -211,7 +211,7 @@ public void CheckStateForStrucutredLogWithGeneralType()
[Fact]
public void CheckStateForExceptionLogged()
{
InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: null);
using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: null);

var exceptionMessage = "Exception Message";
var exception = new Exception(exceptionMessage);
Expand All @@ -234,7 +234,7 @@ public void CheckStateForExceptionLogged()
[Fact]
public void CheckTraceIdForLogWithinDroppedActivity()
{
InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: null);
using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: null);

logger.LogInformation("Log within a dropped activity");
var logRecord = exportedItems[0];
Expand All @@ -248,7 +248,7 @@ public void CheckTraceIdForLogWithinDroppedActivity()
[Fact]
public void CheckTraceIdForLogWithinActivityMarkedAsRecordOnly()
{
InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: null);
using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: null);

var sampler = new RecordOnlySampler();
var exportedActivityList = new List<Activity>();
Expand All @@ -275,7 +275,7 @@ public void CheckTraceIdForLogWithinActivityMarkedAsRecordOnly()
[Fact]
public void CheckTraceIdForLogWithinActivityMarkedAsRecordAndSample()
{
InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: null);
using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: null);

var sampler = new AlwaysOnSampler();
var exportedActivityList = new List<Activity>();
Expand All @@ -302,7 +302,7 @@ public void CheckTraceIdForLogWithinActivityMarkedAsRecordAndSample()
[Fact]
public void VerifyIncludeFormattedMessage_False()
{
InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: options => options.IncludeFormattedMessage = false);
using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: options => options.IncludeFormattedMessage = false);

logger.LogInformation("OpenTelemetry!");
var logRecord = exportedItems[0];
Expand All @@ -312,7 +312,7 @@ public void VerifyIncludeFormattedMessage_False()
[Fact]
public void VerifyIncludeFormattedMessage_True()
{
InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: options => options.IncludeFormattedMessage = true);
using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: options => options.IncludeFormattedMessage = true);

logger.LogInformation("OpenTelemetry!");
var logRecord = exportedItems[0];
Expand All @@ -326,7 +326,7 @@ public void VerifyIncludeFormattedMessage_True()
[Fact]
public void IncludeFormattedMessageTestWhenFormatterNull()
{
InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: options => options.IncludeFormattedMessage = true);
using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: options => options.IncludeFormattedMessage = true);

logger.Log(LogLevel.Information, default, "Hello World!", null, null);
var logRecord = exportedItems[0];
Expand All @@ -346,7 +346,7 @@ public void IncludeFormattedMessageTestWhenFormatterNull()
[Fact]
public void VerifyIncludeScopes_False()
{
InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: options => options.IncludeScopes = false);
using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: options => options.IncludeScopes = false);

using var scope = logger.BeginScope("string_scope");

Expand All @@ -361,7 +361,7 @@ public void VerifyIncludeScopes_False()
[Fact]
public void VerifyIncludeScopes_True()
{
InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: options => options.IncludeScopes = true);
using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: options => options.IncludeScopes = true);

using var scope = logger.BeginScope("string_scope");

Expand Down Expand Up @@ -457,7 +457,7 @@ public void VerifyIncludeScopes_True()
[Fact]
public void VerifyParseStateValues_False_UsingStandardExtensions()
{
InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: options => options.ParseStateValues = false);
using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: options => options.ParseStateValues = false);

// Tests state parsing with standard extensions.

Expand All @@ -471,7 +471,7 @@ public void VerifyParseStateValues_False_UsingStandardExtensions()
[Fact]
public void VerifyParseStateValues_True_UsingStandardExtensions()
{
InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: options => options.ParseStateValues = true);
using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: options => options.ParseStateValues = true);

// Tests state parsing with standard extensions.

Expand Down Expand Up @@ -505,7 +505,7 @@ public void VerifyParseStateValues_True_UsingStandardExtensions()
[Fact]
public void ParseStateValuesUsingStructTest()
{
InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: options => options.ParseStateValues = true);
using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: options => options.ParseStateValues = true);

// Tests struct IReadOnlyList<KeyValuePair<string, object>> parse path.

Expand All @@ -526,7 +526,7 @@ public void ParseStateValuesUsingStructTest()
[Fact]
public void ParseStateValuesUsingListTest()
{
InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: options => options.ParseStateValues = true);
using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: options => options.ParseStateValues = true);

// Tests ref IReadOnlyList<KeyValuePair<string, object>> parse path.

Expand All @@ -547,7 +547,7 @@ public void ParseStateValuesUsingListTest()
[Fact]
public void ParseStateValuesUsingIEnumerableTest()
{
InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: options => options.ParseStateValues = true);
using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: options => options.ParseStateValues = true);

// Tests IEnumerable<KeyValuePair<string, object>> parse path.

Expand All @@ -568,7 +568,7 @@ public void ParseStateValuesUsingIEnumerableTest()
[Fact]
public void ParseStateValuesUsingCustomTest()
{
InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: options => options.ParseStateValues = true);
using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, configure: options => options.ParseStateValues = true);

// Tests unknown state parse path.

Expand All @@ -595,7 +595,7 @@ public void ParseStateValuesUsingCustomTest()
Assert.Same(state, actualState.Value);
}

private static void InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, Action<OpenTelemetryLoggerOptions> configure = null)
private static ILoggerFactory InitializeLoggerFactory(out ILogger logger, out List<LogRecord> exportedItems, Action<OpenTelemetryLoggerOptions> configure = null)
Copy link
Member

Choose a reason for hiding this comment

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

why do we have to have the helper method create a ILogger? It can just do what it claims to do (from the name) - initialize and return a loggerfactory, configured as per the configure action given to it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was meant to simplify the individual tests by removing a dozen lines of identical boilerplate code.
I can move some or all of this into every test, but I don't think that improves the value of the tests themselves.

Copy link
Member

Choose a reason for hiding this comment

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

The name of the helper method did not indicate its intended purpose. The latest commit looks better.
(if we want to achieve less duplication in tests, we can do that, with aptly named helper. but this looks good for me now)

{
exportedItems = new List<LogRecord>();
var exporter = new InMemoryExporter<LogRecord>(exportedItems);
Expand All @@ -611,6 +611,7 @@ private static void InitializeLoggerFactory(out ILogger logger, out List<LogReco
});

logger = loggerFactory.CreateLogger<LogRecordTest>();
return loggerFactory;
}

internal struct Food
Expand Down
14 changes: 7 additions & 7 deletions test/OpenTelemetry.Tests/Logs/LoggerOptionsTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,22 +32,22 @@ public void VerifyOptionsCannotBeChangedAfterInit(bool initialValue)
IncludeScopes = initialValue,
ParseStateValues = initialValue,
};
var processor = new OpenTelemetryLoggerProvider(options);
var provider = new OpenTelemetryLoggerProvider(options);

// Verify initial set
Assert.Equal(initialValue, processor.IncludeFormattedMessage);
Assert.Equal(initialValue, processor.IncludeScopes);
Assert.Equal(initialValue, processor.ParseStateValues);
Assert.Equal(initialValue, provider.IncludeFormattedMessage);
Assert.Equal(initialValue, provider.IncludeScopes);
Assert.Equal(initialValue, provider.ParseStateValues);

// Attempt to change value
options.IncludeFormattedMessage = !initialValue;
options.IncludeScopes = !initialValue;
options.ParseStateValues = !initialValue;

// Verify processor is unchanged
Assert.Equal(initialValue, processor.IncludeFormattedMessage);
Assert.Equal(initialValue, processor.IncludeScopes);
Assert.Equal(initialValue, processor.ParseStateValues);
Assert.Equal(initialValue, provider.IncludeFormattedMessage);
Assert.Equal(initialValue, provider.IncludeScopes);
Assert.Equal(initialValue, provider.ParseStateValues);
}
}
}
Expand Down