Skip to content

Extend ConsoleExporter to add support for Logs #1438

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
cijothomas merged 3 commits into from
Nov 3, 2020
Merged

Extend ConsoleExporter to add support for Logs #1438

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f3d3e1c
Added support for Logs; Removed disaplayAsJson option from ConsoleExp…
utpilla Nov 2, 2020
c91a432
Print LogRecord exception on console
utpilla Nov 2, 2020
0707ee2
Merge branch 'master' into utpilla/Update-ConsoleExporter-For-LogRecord
cijothomas Nov 3, 2020
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: 0 additions & 6 deletions examples/Console/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,22 +116,16 @@ internal class ZPagesOptions
[Verb("console", HelpText = "Specify the options required to test console exporter")]
internal class ConsoleOptions
{
[Option('p', "displayasjson", HelpText = "Specify if the output should be displayed as json or not (default: false)", Default = false)]
public bool DisplayAsJson { get; set; }
}

[Verb("otelshim", HelpText = "Specify the options required to test OpenTelemetry Shim with console exporter")]
internal class OpenTelemetryShimOptions
{
[Option('p', "displayasjson", HelpText = "Specify if the output should be displayed as json or not (default: false)", Default = false)]
public bool DisplayAsJson { get; set; }
}

[Verb("opentracing", HelpText = "Specify the options required to test OpenTracing Shim with console exporter")]
internal class OpenTracingShimOptions
{
[Option('p', "displayasjson", HelpText = "Specify if the output should be displayed as json or not (default: false)", Default = false)]
public bool DisplayAsJson { get; set; }
}

[Verb("otlp", HelpText = "Specify the options required to test OpenTelemetry Protocol (OTLP)")]
Expand Down
2 changes: 1 addition & 1 deletion examples/Console/TestConsoleExporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ internal static object Run(ConsoleOptions options)
.AddSource("MyCompany.MyProduct.MyWebServer")
.SetResource(Resources.CreateServiceResource("MyServiceName"))
.AddProcessor(new MyProcessor()) // This must be added before ConsoleExporter
.AddConsoleExporter(opt => opt.DisplayAsJson = options.DisplayAsJson)
.AddConsoleExporter()
.Build();

// The above line is required only in applications
Expand Down
2 changes: 1 addition & 1 deletion examples/Console/TestHttpClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ internal static object Run()
.AddHttpClientInstrumentation()
.SetResource(Resources.CreateServiceResource("http-service-example"))
.AddSource("http-client-test")
.AddConsoleExporter(opt => opt.DisplayAsJson = false)
.AddConsoleExporter()
.Build();

var source = new ActivitySource("http-client-test");
Expand Down
2 changes: 1 addition & 1 deletion examples/Console/TestOTelShimWithConsoleExporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ internal static object Run(OpenTelemetryShimOptions options)
using var tracerProvider = Sdk.CreateTracerProviderBuilder()
.AddSource("MyCompany.MyProduct.MyWebServer")
.SetResource(Resources.CreateServiceResource("MyServiceName"))
.AddConsoleExporter(opt => opt.DisplayAsJson = options.DisplayAsJson)
.AddConsoleExporter()
.Build();

// The above line is required only in applications
Expand Down
2 changes: 1 addition & 1 deletion examples/Console/TestOpenTracingWithConsoleExporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ internal static object Run(OpenTracingShimOptions options)
using var openTelemetry = Sdk.CreateTracerProviderBuilder()
.AddSource("MyCompany.MyProduct.MyWebServer")
.SetResource(Resources.CreateServiceResource("MyServiceName"))
.AddConsoleExporter(opt => opt.DisplayAsJson = options.DisplayAsJson)
.AddConsoleExporter()
.Build();

// The above line is required only in applications
Expand Down
36 changes: 0 additions & 36 deletions src/OpenTelemetry.Exporter.Console/ActivitySpanIdConverter.cs
View file
Open in desktop

This file was deleted.

36 changes: 0 additions & 36 deletions src/OpenTelemetry.Exporter.Console/ActivityTraceIdConverter.cs
View file
Open in desktop

This file was deleted.

56 changes: 33 additions & 23 deletions src/OpenTelemetry.Exporter.Console/ConsoleExporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,42 +17,28 @@
using System;
using System.Diagnostics;
using System.Linq;
using System.Text.Json;
using System.Text.Json.Serialization;
#if NET461 || NETSTANDARD2_0
using OpenTelemetry.Logs;
#endif
using OpenTelemetry.Resources;
using OpenTelemetry.Trace;

namespace OpenTelemetry.Exporter
{
public class ConsoleExporter : BaseExporter<Activity>
public class ConsoleExporter<T> : BaseExporter<T>

@CodeBlanch CodeBlanch Nov 3, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So now I can do ConsoleExporter<some_custom_type> and it will emit nothing? I don't see a lot of value in having the generic, TBH. Why don't we just have ConsoleLogExporter : BaseExporter<LogRecord> & ConsoleActivityExporter : BaseExporter<Activity>? There's really not that much code being duplicated.

@CodeBlanch CodeBlanch Nov 3, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

OR how about something like...

public class ConsoleExporter<T> : BaseExporter<T>
{
   public ConsoleExporter(Action<T> writeToConsole) { ... }

   public override ExportResult Export(in Batch<T> batch)
   {
      foreach (var item in batch) {
         this.writeToConsole(item);
     }
   }
}

public class ConsoleLogExporter : ConsoleExporter<LogRecord>
{
   public ConsoleLogExporter () : base(WriteLogToConsole) { }

   private static void WriteLogToConsole(LogRecord logRecord) {
      var rightPaddingLength = 30;

      Console.WriteLine($"{"LogRecord.TraceId:".PadRight(rightPaddingLength)}{logRecord.TraceId}");
      Console.WriteLine($"{"LogRecord.SpanId:".PadRight(rightPaddingLength)}{logRecord.SpanId}");
      Console.WriteLine($"{"LogRecord.Timestamp:".PadRight(rightPaddingLength)}{logRecord.Timestamp:yyyy-MM-ddTHH:mm:ss.fffffffZ}");
      Console.WriteLine($"{"LogRecord.EventId:".PadRight(rightPaddingLength)}{logRecord.EventId}");
      Console.WriteLine($"{"LogRecord.CategoryName:".PadRight(rightPaddingLength)}{logRecord.CategoryName}");
      Console.WriteLine($"{"LogRecord.LogLevel:".PadRight(rightPaddingLength)}{logRecord.LogLevel}");
      Console.WriteLine($"{"LogRecord.TraceFlags:".PadRight(rightPaddingLength)}{logRecord.TraceFlags}");
      Console.WriteLine($"{"LogRecord.State:".PadRight(rightPaddingLength)}{logRecord.State}");
      if (logRecord.Exception is { })
      {
         Console.WriteLine($"{"LogRecord.Exception:".PadRight(rightPaddingLength)}{logRecord.Exception?.Message}");
      }

      Console.WriteLine();
   }
}

@utpilla utpilla Nov 3, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't think you can add an Exporter of a type anything other than Activity or LogRecord. To add an exporter for traces, you have to use the TracerProviderBuilder AddProcessor(BaseProcessor processor) method of TracerProviderBuilder class. Here, both SimpleExportProcessor and BatchExportProcessor will enforce you to only use Activity as the type.

Similarly, to add an exporter to Logs, you have to use the extension method OpenTelemetryLoggerOptions AddProcessor(BaseProcessor processor). Here, both SimpleExportProcessor and BatchExportProcessor will enforce you to only use LogRecord as the type.

@CodeBlanch CodeBlanch Nov 4, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That's a good point. I still feel like it is misusing generics and we could improve it though. If it was internal, no prob, but it is part of the public API so we'll have to live with it forever.

@cijothomas @reyang Any thoughts?

@reyang reyang Nov 4, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No strong opinion from my side.

I agree that having a switch-case (or if-else) in generic method (e.g. if T is Activity do this, if T is LogRecord do that) is not very idiomatic / performant.

where T : class
{
private readonly JsonSerializerOptions serializerOptions;
private readonly bool displayAsJson;

public ConsoleExporter(ConsoleExporterOptions options)
{
this.serializerOptions = new JsonSerializerOptions()
{
WriteIndented = true,
};

this.displayAsJson = options?.DisplayAsJson ?? false;

this.serializerOptions.Converters.Add(new JsonStringEnumConverter());
this.serializerOptions.Converters.Add(new ActivitySpanIdConverter());
this.serializerOptions.Converters.Add(new ActivityTraceIdConverter());
}

public override ExportResult Export(in Batch<Activity> batch)
public override ExportResult Export(in Batch<T> batch)
{
foreach (var activity in batch)
if (typeof(T) == typeof(Activity))

@CodeBlanch CodeBlanch Nov 3, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: Would prob be more efficient to check the type in ctor and then set a field instead of checking every batch.

{
if (this.displayAsJson)
{
Console.WriteLine(JsonSerializer.Serialize(activity, this.serializerOptions));
}
else
foreach (var item in batch)
{
var activity = item as Activity;
Console.WriteLine($"Activity.Id: {activity.Id}");
if (!string.IsNullOrEmpty(activity.ParentId))
{
Expand Down Expand Up @@ -123,6 +109,30 @@ public override ExportResult Export(in Batch<Activity> batch)
Console.WriteLine();
}
}
#if NET461 || NETSTANDARD2_0
else if (typeof(T) == typeof(LogRecord))
{
var rightPaddingLength = 30;
foreach (var item in batch)
{
var logRecord = item as LogRecord;
Console.WriteLine($"{"LogRecord.TraceId:".PadRight(rightPaddingLength)}{logRecord.TraceId}");

@reyang reyang Nov 3, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: the output is a bit different from the Activity one (e.g. here we have the LogRecord.Something).
Might be nice to make them consistent. Not blocking though.

@utpilla utpilla Nov 3, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's LogRecord.propertyName right now, what you suggest it be? Should we use "Activity" instead of "LogRecord"?

@reyang reyang Nov 3, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would you help to update the example to use the console exporter?

opentelemetry-dotnet/docs/logs/getting-started/Program.cs

Line 33 in 46094ad

.AddInMemoryExporter()); // TODO: change to console output

We can use it to drive the output format discussion.

Console.WriteLine($"{"LogRecord.SpanId:".PadRight(rightPaddingLength)}{logRecord.SpanId}");
Console.WriteLine($"{"LogRecord.Timestamp:".PadRight(rightPaddingLength)}{logRecord.Timestamp:yyyy-MM-ddTHH:mm:ss.fffffffZ}");
Console.WriteLine($"{"LogRecord.EventId:".PadRight(rightPaddingLength)}{logRecord.EventId}");
Console.WriteLine($"{"LogRecord.CategoryName:".PadRight(rightPaddingLength)}{logRecord.CategoryName}");
Console.WriteLine($"{"LogRecord.LogLevel:".PadRight(rightPaddingLength)}{logRecord.LogLevel}");
Console.WriteLine($"{"LogRecord.TraceFlags:".PadRight(rightPaddingLength)}{logRecord.TraceFlags}");
Console.WriteLine($"{"LogRecord.State:".PadRight(rightPaddingLength)}{logRecord.State}");
if (logRecord.Exception is { })
{
Console.WriteLine($"{"LogRecord.Exception:".PadRight(rightPaddingLength)}{logRecord.Exception?.Message}");
}

Console.WriteLine();
}
}
#endif

return ExportResult.Success;
}
Expand Down
2 changes: 1 addition & 1 deletion src/OpenTelemetry.Exporter.Console/ConsoleExporterHelperExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public static TracerProviderBuilder AddConsoleExporter(this TracerProviderBuilde

var options = new ConsoleExporterOptions();
configure?.Invoke(options);
return builder.AddProcessor(new SimpleExportProcessor<Activity>(new ConsoleExporter(options)));
return builder.AddProcessor(new SimpleExportProcessor<Activity>(new ConsoleExporter<Activity>(options)));
}
}
}
1 change: 0 additions & 1 deletion src/OpenTelemetry.Exporter.Console/ConsoleExporterOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,5 @@ namespace OpenTelemetry.Exporter
{
public class ConsoleExporterOptions
{
public bool DisplayAsJson { get; set; }
}
}
8 changes: 2 additions & 6 deletions src/OpenTelemetry.Exporter.Console/OpenTelemetry.Exporter.Console.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFrameworks>net461;netstandard2.0</TargetFrameworks>
<TargetFrameworks>net452;net461;netstandard2.0</TargetFrameworks>
<Description>Console exporter for OpenTelemetry .NET</Description>
<PackageTags>$(PackageTags);Console;distributed-tracing</PackageTags>
</PropertyGroup>
Expand All @@ -14,8 +14,4 @@
<ProjectReference Include="$(RepoRoot)\src\OpenTelemetry\OpenTelemetry.csproj" />
</ItemGroup>

<ItemGroup>
<PackageReference Include="System.Text.Json" Version="$(SystemTextJsonPkgVer)" />
</ItemGroup>

</Project>
10 changes: 2 additions & 8 deletions src/OpenTelemetry.Exporter.Console/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,9 @@ environment.
dotnet add package OpenTelemetry.Exporter.Console
```

## Configuration

You can configure the `ConsoleExporter` by following the directions below:

* `DisplayAsJson`: Boolean to show data as JSON.

See the
[`TestConsoleExporter.cs`](../../examples/Console/TestConsoleExporter.cs)
for an example of how to use the exporter.
[`TestConsoleExporter.cs`](../../examples/Console/TestConsoleExporter.cs) for an
example of how to use the exporter.

## References

Expand Down
⚡