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

Use TagTransformer for ConsoleExporter 💻 #3311

Merged

Conversation

alanwest
Copy link
Member

Addressing #3229 for the ConsoleExporter

@alanwest alanwest requested a review from a team May 27, 2022 23:23
this.WriteLine($"{string.Empty,-4}{logRecord.StateValues[i].Key,-RightPaddingLength}{logRecord.StateValues[i].Value}");
this.WriteLine($"{string.Empty,-4}{result}");
Copy link
Member Author

@alanwest alanwest May 27, 2022

Choose a reason for hiding this comment

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

Lost some formatting of alignment in a number of spots. If folks feel strongly about preserving this formatting, I had it on my mind to make an override of TryTransformTag that took a format string or the likes.

Before
LogRecord.Timestamp:               2022-05-27T22:19:45.7034956Z
LogRecord.TraceId:                 5040b69ef7389abf3edfeee5c9583dd2
LogRecord.SpanId:                  fdf9c630875f7415
LogRecord.TraceFlags:              Recorded
LogRecord.CategoryName:            Examples.AspNetCore.Controllers.WeatherForecastController
LogRecord.LogLevel:                Information
LogRecord.FormattedMessage:        WeatherForecasts generated 5: Examples.AspNetCore.WeatherForecast, Examples.AspNetCore.WeatherForecast, Examples.AspNetCore.WeatherForecast, Examples.AspNetCore.WeatherForecast, Examples.AspNetCore.WeatherForecast
LogRecord.StateValues (Key:Value):
    count                              5
    forecasts                          Examples.AspNetCore.WeatherForecast[]
    OriginalFormat (a.k.a. Body)       WeatherForecasts generated {count}: {forecasts}
LogRecord.ScopeValues (Key:Value):
[Scope.0]:SpanId                             fdf9c630875f7415
[Scope.0]:TraceId                            5040b69ef7389abf3edfeee5c9583dd2
[Scope.0]:ParentId                           0000000000000000
[Scope.1]:ConnectionId                       0HMI05RCLU4BE
[Scope.2]:RequestId                          0HMI05RCLU4BE:00000003
[Scope.2]:RequestPath                        /WeatherForecast
[Scope.3]:ActionId                           e810ba66-bead-4b98-aef9-f7d726b73a4f
[Scope.3]:ActionName                         Examples.AspNetCore.Controllers.WeatherForecastController.Get (Examples.AspNetCore)
[Scope.4]:Id                                 bbbe3422157247edab47fc32f1807b5d
[Scope.4]:{OriginalFormat}                   {Id}

Resource associated with LogRecord:
service.name: AspNetCoreExampleService
service.version: 1.0.0.0
service.instance.id: awest-win10
After
LogRecord.Timestamp:               2022-05-27T22:53:02.4762430Z
LogRecord.TraceId:                 7e8a593691b28893d4ca0e248845ea15
LogRecord.SpanId:                  5afcebdead5d82f9
LogRecord.TraceFlags:              Recorded
LogRecord.CategoryName:            Examples.AspNetCore.Controllers.WeatherForecastController
LogRecord.LogLevel:                Information
LogRecord.FormattedMessage:        WeatherForecasts generated 5: Examples.AspNetCore.WeatherForecast, Examples.AspNetCore.WeatherForecast, Examples.AspNetCore.WeatherForecast, Examples.AspNetCore.WeatherForecast, Examples.AspNetCore.WeatherForecast
LogRecord.StateValues (Key:Value):
    count: 5
    forecasts: ["Examples.AspNetCore.WeatherForecast","Examples.AspNetCore.WeatherForecast","Examples.AspNetCore.WeatherForecast","Examples.AspNetCore.WeatherForecast","Examples.AspNetCore.WeatherForecast"]
    {OriginalFormat}: WeatherForecasts generated {count}: {forecasts}
LogRecord.ScopeValues (Key:Value):
[Scope.0]:SpanId: 5afcebdead5d82f9
[Scope.0]:TraceId: 7e8a593691b28893d4ca0e248845ea15
[Scope.0]:ParentId: 0000000000000000
[Scope.1]:ConnectionId: 0HMI06EJIP8VD
[Scope.2]:RequestId: 0HMI06EJIP8VD:00000001
[Scope.2]:RequestPath: /WeatherForecast
[Scope.3]:ActionId: 4bfb0faf-fa8c-47bb-9350-e62957f1d350
[Scope.3]:ActionName: Examples.AspNetCore.Controllers.WeatherForecastController.Get (Examples.AspNetCore)
[Scope.4]:Id: e13f37de769345ceb76f3630ac478598
[Scope.4]:{OriginalFormat}: {Id}

Resource associated with LogRecord:
service.name: AspNetCoreExampleService
service.version: 1.0.0.0
service.instance.id: awest-win10

Copy link
Member

Choose a reason for hiding this comment

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

I actually prefer that we remove all alignments.
For folks who want better alignments, we might consider an option to output markdown tables (like how Benchmark.NET is doing).

Copy link
Member

Choose a reason for hiding this comment

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

I think the current output format is very similar to YAML, maybe we can move towards that direction? 😆

@codecov
Copy link

codecov bot commented May 27, 2022

Codecov Report

Merging #3311 (6b0ac56) into main (b045253) will increase coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3311      +/-   ##
==========================================
+ Coverage   85.39%   85.42%   +0.03%     
==========================================
  Files         269      269              
  Lines        9552     9552              
==========================================
+ Hits         8157     8160       +3     
+ Misses       1395     1392       -3     
Impacted Files Coverage Δ
...emetry.Api/Internal/OpenTelemetryApiEventSource.cs 76.47% <0.00%> (-5.89%) ⬇️
...metryProtocol/Implementation/ActivityExtensions.cs 95.60% <0.00%> (+1.09%) ⬆️
src/OpenTelemetry/BatchExportProcessor.cs 81.90% <0.00%> (+2.85%) ⬆️

@cijothomas cijothomas merged commit f314948 into open-telemetry:main May 31, 2022
@alanwest alanwest deleted the alanwest/console-exporter-tags branch May 31, 2022 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants