Use TraceLogger as default logger in ASP.NET Full Framework #1288
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit adds a logger implementation, TraceLogger, that writes agent logs to a TraceSource with name "Elastic.Apm". This logger is configured as the default logger for ASP.NET Full Framework applications, which can use configuration to write log messages to file, debug window, Windows event log, etc. Add a section to docs with an example of how to configure the logger in web.config. Move the default log level from ConsoleLogger into DefaultValues. Closes elastic#1263
💔 Tests Failed
Expand to view the summary
Build stats
Test stats 🧪
Trends 🧪Test errors
Expand to view the tests failures> Show only the first 10 test failures
|
gregkalapos
reviewed
May 5, 2021
| /// </summary> | ||
| internal class TraceLogger : IApmLogger, ILogLevelSwitchable | ||
| { | ||
| private const string SourceName = "Elastic.Apm"; |
There was a problem hiding this comment.
Suggested change
| private const string SourceName = "Elastic.Apm"; | |
| private const string _sourceName= "Elastic.Apm"; | |
|
|
||
| var message = formatter(state, e); | ||
|
|
||
| // default message size is 51 + length of loglevel (max 8), message and exception. |
There was a problem hiding this comment.
I don't really get this comment. What do you mean by "default message size is 51"?
Looking at the code I'd think that the strategy here is to start with a fairly small number for the capacity, but then ultimately the capacity of the StringBuilder will grow based on the length of the messages we print. So in this case we start with 80 (line below) and then if the message is longer then StringBuilder will increase the capacity and once we reuse it for a later massage the capacity will be bigger. So it'll adapt to the longest message. Do I understand this correctly?
What I wanna say here is that I don't really get the comment here :)
There was a problem hiding this comment.
It means that the complete log message size at a minimum will be 51 characters, plus the variable length of log level (max 8 characters), message and exception. So any starting capacity should at a minimum accommodate this, which the starting capacity of 80 characters will - assuming no exception and max log level length, leaves 21 characters for the message. This may be overly conservative, in which case a better estimate may be to use 59 + the calculated mean length of a log message.
StringBuilder will grow (and allocate) to accommodate larger log messages. The intention is to avoid allocating whilst serving the common usage.
There was a problem hiding this comment.
Got it - the term default message size was strange to me, but I got it.
russcam
added a commit
to russcam/apm-agent-dotnet
that referenced
this pull request
May 5, 2021
This commit removes the StringBuilderCache, calculating a capacity for a StringBuilder from the log component lengths.
gregkalapos
approved these changes
May 26, 2021
|
Removed |
v1v
added a commit
to v1v/apm-agent-dotnet
that referenced
this pull request
Jun 8, 2021
…u-20 * upstream/master: (21 commits) Prefer W3C traceparent over elastic-apm-traceparent (elastic#1302) fix spacing and cross references in docs (elastic#1328) Update README (elastic#1325) Mark MicrosoftAzureBlobStorageTracer internal (elastic#1326) Update docs (elastic#1327) Update context.destination.address (elastic#1324) synchronize json schema specs (elastic#1320) Don't package Elastic.Apm.Specification (elastic#1316) Update setup.asciidoc (elastic#1318) Prepare release v.1.10.0 (elastic#1314) Fix nullref in Elastic.Apm.Extensions.Logging (elastic#1311) Capture errors with startup hook auto instrumentation (elastic#1298) Use Logger to log exception in AgentComponents initialization (elastic#1305) fix: use .NET native SDK for build and test (elastic#1301) Skip running Elasticsearch docker test when docker not available (elastic#1312) Use TraceLogger as default logger in ASP.NET Full Framework (elastic#1288) Create receive messaging span when inside transaction (elastic#1308) Fix SanitizeFieldNamesTests (elastic#1299) Do not capture HTTP child spans for Elasticsearch (elastic#1306) Use storage account in destination.service.resource (elastic#1284) ...
russcam
added a commit
that referenced
this pull request
Jun 10, 2021
russcam
added a commit
that referenced
this pull request
Jun 10, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit adds a logger implementation, TraceLogger, that
writes agent logs to a TraceSource with name "Elastic.Apm".
This logger is configured as the default logger for
ASP.NET Full Framework applications, which can use configuration
to write log messages to file, debug window, Windows event log, etc.
Add a section to docs with an example of how to configure
the logger in web.config.
Move the default log level from ConsoleLogger into DefaultValues.
Closes #1263