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

Logging improvements #456

Merged
merged 6 commits into from
Apr 11, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
6 changes: 3 additions & 3 deletions LICENSE-3rdparty.csv
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Component,Origin,License,Copyright
Datadog.Trace,https://github.com/serilog/serilog,Apache-2.0,Copyright (c) 2013-2018 Serilog Contributors
Datadog.Trace,https://github.com/serilog/serilog-sinks-file,Apache-2.0,Copyright (c) 2016 Serilog Contributors
Datadog.Trace,https://github.com/JamesNK/Newtonsoft.Json,MIT,Copyright (c) 2007 James Newton-King
OpenTelemetry.AutoInstrumentation,https://github.com/serilog/serilog,Apache-2.0,Copyright (c) 2013-2018 Serilog Contributors
OpenTelemetry.AutoInstrumentation,https://github.com/serilog/serilog-sinks-file,Apache-2.0,Copyright (c) 2016 Serilog Contributors
OpenTelemetry.AutoInstrumentation,https://github.com/JamesNK/Newtonsoft.Json,MIT,Copyright (c) 2007 James Newton-King
OpenTelemetry.AutoInstrumentation.Native,https://github.com/dotnet/runtime,MIT,Copyright (c) .NET Foundation and contributors. All rights reserved.
OpenTelemetry.AutoInstrumentation.Native,https://github.com/Microsoft/clr-samples,MIT,Copyright (c) .NET Foundation and contributors. All rights reserved.
OpenTelemetry.AutoInstrumentation.Native,https://github.com/MicrosoftArchive/clrprofiler,MIT,Copyright (c) Microsoft Corporation. All rights reserved.
Expand Down
14 changes: 14 additions & 0 deletions src/OpenTelemetry.AutoInstrumentation.Loader/FileSink.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,20 @@
// This comment is here to prevent StyleCop from analyzing a file originally from Serilog.
//------------------------------------------------------------------------------

// Copyright 2013-2019 Serilog Contributors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

using System;
using System.IO;
using System.Text;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ private static string SetStartupLogFilePath()

try
{
var process = Process.GetCurrentProcess();
using var process = Process.GetCurrentProcess();
// Do our best to not block other processes on write
return Path.Combine(LogDirectory, $"dotnet-tracer-loader-{process.ProcessName}-{process.Id}.log");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ internal class IntegrationMapper
private const string EndMethodName = "OnMethodEnd";
private const string EndAsyncMethodName = "OnAsyncMethodEnd";

private static readonly ILogger Log = ConsoleLogger.Create(typeof(IntegrationMapper));
private static readonly ILogger Log = OtelLogging.GetLogger();
private static readonly MethodInfo UnwrapReturnValueMethodInfo = typeof(IntegrationMapper).GetMethod(nameof(IntegrationMapper.UnwrapReturnValue), BindingFlags.NonPublic | BindingFlags.Static);
private static readonly MethodInfo ConvertTypeMethodInfo = typeof(IntegrationMapper).GetMethod(nameof(IntegrationMapper.ConvertType), BindingFlags.NonPublic | BindingFlags.Static);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ namespace OpenTelemetry.AutoInstrumentation.CallTarget.Handlers;

internal static class IntegrationOptions<TIntegration, TTarget>
{
private static readonly ILogger Log = ConsoleLogger.Create(typeof(IntegrationOptions<TIntegration, TTarget>));
private static readonly ILogger Log = OtelLogging.GetLogger();

private static volatile bool _disableIntegration = false;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,21 @@ public class ConfigurationKeys
/// </summary>
public const string Http2UnencryptedSupportEnabled = "OTEL_DOTNET_AUTO_HTTP2UNENCRYPTEDSUPPORT_ENABLED";

/// <summary>
/// Configuration key for setting the directory for the profiler's log files.
/// If set, this setting takes precedence over environment variable OTEL_DOTNET_AUTO_LOG_PATH.
/// If not set, default is
/// "%ProgramData%"\OpenTelemetry .NET AutoInstrumentation\logs\" on Windows or
/// "/var/log/opentelemetry/dotnet/" on Linux.
/// </summary>
public const string LogDirectory = "OTEL_DOTNET_AUTO_LOG_DIRECTORY";

/// <summary>
/// Configuration key for setting the path for the profiler's native log file.
/// Environment variable OTEL_DOTNET_AUTO_LOG_DIRECTORY takes precedence over this setting, if set.
/// </summary>
public const string LogPath = "OTEL_DOTNET_AUTO_LOG_PATH";

Comment on lines +87 to +101
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to have both settings?

Copy link
Contributor Author

@lachmatt lachmatt Apr 12, 2022

Choose a reason for hiding this comment

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

I added them both here, because both need to be checked to be consistent with StartupLogger, which takes them into account when deciding on log directory.

Copy link
Member

@pellared pellared Apr 12, 2022

Choose a reason for hiding this comment

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

I guess it is also used in the native logger. I created an issue to remove one of them: #476

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, better to have a single env var and use the same for native and managed.

Another thing that we may want to consider post-beta is: the native log is one file for multiple applications so it is not symmetrical to the managed one. Perhaps it will make the user's life simpler if the logs of a single run were easier to "group".

/// <summary>
/// String format patterns used to match integration-specific configuration keys.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

using System;
using System.Runtime.CompilerServices;
using OpenTelemetry.AutoInstrumentation.Logging;

namespace OpenTelemetry.AutoInstrumentation.DuckTyping;

Expand All @@ -25,8 +24,6 @@ namespace OpenTelemetry.AutoInstrumentation.DuckTyping;
/// </summary>
public static class DuckTypeExtensions
{
private static readonly ILogger Log = ConsoleLogger.Create(typeof(DuckType));

/// <summary>
/// Gets the duck type instance for the object implementing a base class or interface T
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ namespace OpenTelemetry.AutoInstrumentation;

internal partial class FrameworkDescription
{
private static readonly ILogger Log = ConsoleLogger.Create(typeof(FrameworkDescription));
private static readonly ILogger Log = OtelLogging.GetLogger();

private static readonly Assembly RootAssembly = typeof(object).Assembly;

Expand Down
31 changes: 14 additions & 17 deletions src/OpenTelemetry.AutoInstrumentation/Instrumentation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,13 @@
// </copyright>

using System;
using System.Diagnostics;
using System.Threading;
using OpenTelemetry.AutoInstrumentation.Configuration;
using OpenTelemetry.AutoInstrumentation.Logging;
using OpenTelemetry.Context.Propagation;
using OpenTelemetry.Shims.OpenTracing;
using OpenTelemetry.Trace;
using OpenTracing.Util;

namespace OpenTelemetry.AutoInstrumentation;

Expand All @@ -29,7 +30,8 @@ namespace OpenTelemetry.AutoInstrumentation;
/// </summary>
public static class Instrumentation
{
private static readonly Process _process = Process.GetCurrentProcess();
private static readonly ILogger Logger = OtelLogging.GetLogger();

private static int _firstInitialization = 1;
private static int _isExiting = 0;

Expand Down Expand Up @@ -81,7 +83,7 @@ public static void Initialize()
.InvokePlugins(TracerSettings.TracerPlugins);

_tracerProvider = builder.Build();
Log("OpenTelemetry tracer initialized.");
Logger.Information("OpenTelemetry tracer initialized.");

// Register to shutdown events
AppDomain.CurrentDomain.ProcessExit += OnExit;
Expand All @@ -91,7 +93,7 @@ public static void Initialize()
}
catch (Exception ex)
{
Log($"OpenTelemetry SDK load exception: {ex}");
Logger.Error(ex, "OpenTelemetry SDK load exception.");
throw;
}

Expand All @@ -107,17 +109,17 @@ public static void Initialize()

// This registration must occur prior to any reference to the OpenTracing tracer:
// otherwise the no-op tracer is going to be used by OpenTracing instead.
OpenTracing.Util.GlobalTracer.RegisterIfAbsent(openTracingShim);
Log("OpenTracingShim loaded.");
GlobalTracer.RegisterIfAbsent(openTracingShim);
Logger.Information("OpenTracingShim loaded.");
}
else
{
Log("OpenTracingShim was not loaded as the provider is not initialized.");
Logger.Information("OpenTracingShim was not loaded as the provider is not initialized.");
}
}
catch (Exception ex)
{
Log($"OpenTracingShim exception: {ex}");
Logger.Error(ex, "OpenTracingShim exception.");
throw;
}
}
Expand All @@ -134,13 +136,13 @@ private static void OnExit(object sender, EventArgs e)
{
_tracerProvider.Dispose();

Log("OpenTelemetry tracer exit.");
Logger.Information("OpenTelemetry tracer exit.");
}
catch (Exception ex)
{
try
{
Log($"An error occured while attempting to exit. {ex}");
Logger.Error(ex, "An error occured while attempting to exit.");
}
catch
{
Expand All @@ -156,15 +158,15 @@ private static void OnUnhandledException(object sender, UnhandledExceptionEventA
{
if (args.IsTerminating)
{
Log("UnhandledException event raised with a terminating exception.");
Logger.Error("UnhandledException event raised with a terminating exception.");
OnExit(sender, args);
}
}
catch (Exception ex)
{
try
{
Log($"An exception occured while processing an unhandled exception. {ex}");
Logger.Error(ex, "An exception occured while processing an unhandled exception.");
}
catch
{
Expand All @@ -173,9 +175,4 @@ private static void OnUnhandledException(object sender, UnhandledExceptionEventA
}
}
}

private static void Log(string message)
{
Console.WriteLine($">>>>>>>>>>>>>>>>>>>>>>> Process: {_process.ProcessName}({_process.Id}): {message}");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ internal class GraphQLCommon
internal static readonly ActivitySource ActivitySource = new ActivitySource(
"OpenTelemetry.AutoInstrumentation.GraphQL", "0.0.1");

private static readonly ILogger Log = ConsoleLogger.Create(typeof(GraphQLCommon));
pjanotti marked this conversation as resolved.
Show resolved Hide resolved
private static readonly ILogger Log = OtelLogging.GetLogger();

internal static Activity CreateActivityFromValidate(IDocument document)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public static class MongoDbIntegration

private const string OperationName = "mongodb.query";

private static readonly ILogger Log = ConsoleLogger.Create(typeof(MongoDbIntegration));
private static readonly ILogger Log = OtelLogging.GetLogger();

private static readonly ActivitySource ActivitySource = new ActivitySource(
"OpenTelemetry.AutoInstrumentation.MongoDB", "0.0.1");
Expand Down
78 changes: 78 additions & 0 deletions src/OpenTelemetry.AutoInstrumentation/Logging/FileSink.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
//------------------------------------------------------------------------------
// <auto-generated />
// This comment is here to prevent StyleCop from analyzing a file originally from Serilog.
//------------------------------------------------------------------------------

// Copyright 2013-2019 Serilog Contributors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

using System;
using System.IO;
using System.Text;
using OpenTelemetry.AutoInstrumentation.Logging;

namespace OpenTelemetry.AutoInstrumentation.Logging
{
internal sealed class FileSink : ISink, IDisposable
{
readonly TextWriter _output;
readonly FileStream _underlyingStream;
readonly WriteCountingStream _countingStreamWrapper;
readonly object _syncRoot = new object();
static readonly long FileSizeLimitBytes = 10 * 1024 * 1024;
pellared marked this conversation as resolved.
Show resolved Hide resolved

public FileSink(string path, Encoding encoding = null)
{
if (path == null) throw new ArgumentNullException(nameof(path));

var directory = Path.GetDirectoryName(path);
if (!string.IsNullOrWhiteSpace(directory) && !Directory.Exists(directory))
{
Directory.CreateDirectory(directory);
}

_underlyingStream = System.IO.File.Open(path, FileMode.Append, FileAccess.Write, FileShare.Read);

Stream outputStream = _countingStreamWrapper = new WriteCountingStream(_underlyingStream);
_output = new StreamWriter(outputStream, encoding ?? new UTF8Encoding(encoderShouldEmitUTF8Identifier: false));
}

public void Write(string message)
{
lock (_syncRoot)
{
if (_countingStreamWrapper.CountedLength >= FileSizeLimitBytes)
{
return;
}
_output.Write(message);
FlushToDisk();
}
}

public void Dispose()
{
lock (_syncRoot)
{
_output.Dispose();
}
}

private void FlushToDisk()
{
_output.Flush();
_underlyingStream.Flush(true);
}
}
}
22 changes: 22 additions & 0 deletions src/OpenTelemetry.AutoInstrumentation/Logging/ISink.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// <copyright file="ISink.cs" company="OpenTelemetry Authors">
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// </copyright>

namespace OpenTelemetry.AutoInstrumentation.Logging;

internal interface ISink
{
public void Write(string message);
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// <copyright file="ConsoleLogger.cs" company="OpenTelemetry Authors">
// <copyright file="Logger.cs" company="OpenTelemetry Authors">
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
Expand All @@ -19,25 +19,14 @@

namespace OpenTelemetry.AutoInstrumentation.Logging;

internal class ConsoleLogger : ILogger
internal class Logger : ILogger
{
private static readonly object[] NoPropertyValues = Array.Empty<object>();
private readonly ISink _sink;

private readonly string _name;

public ConsoleLogger(string name)
{
_name = name;
}

public static ConsoleLogger Create<T>()
internal Logger(ISink sink)
{
return Create(typeof(T));
}

public static ConsoleLogger Create(Type type)
{
return new ConsoleLogger(type.Name);
_sink = sink ?? throw new ArgumentNullException(nameof(sink));
}

public bool IsEnabled(LogLevel level)
Expand Down Expand Up @@ -206,11 +195,14 @@ private void WriteImpl(LogLevel level, Exception exception, string messageTempla
{
try
{
Console.WriteLine($"[{_name}] {level}:{string.Format(messageTemplate, args)}");
var message =
zacharycmontoya marked this conversation as resolved.
Show resolved Hide resolved
$"[{DateTime.UtcNow}] [{level}] {messageTemplate} {Environment.NewLine}";
_sink.Write(message);

if (exception != null)
{
Console.WriteLine($">> Exception: {exception.Message} {Environment.NewLine} {exception}");
var exceptionMessage = $"Exception: {exception.Message}{Environment.NewLine}{exception}{Environment.NewLine}";
_sink.Write(exceptionMessage);
}
}
catch
Expand All @@ -221,7 +213,6 @@ private void WriteImpl(LogLevel level, Exception exception, string messageTempla
var properties = args.Length == 0
? string.Empty
: "; " + string.Join(", ", args);

Console.Error.WriteLine($"{messageTemplate}{properties}{ex}");
}
catch
Expand Down
Loading