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

EventSourceLogger does not propagate state data during ActivityStart #33486

Open
wiktork opened this issue Mar 11, 2020 · 9 comments
Open

EventSourceLogger does not propagate state data during ActivityStart #33486

wiktork opened this issue Mar 11, 2020 · 9 comments

Comments

@wiktork
Copy link
Member

wiktork commented Mar 11, 2020

EventSourceLogger only sends IReadOnlyList<KeyValuePair<string, object>> state objects as part of the ActivityStart payload. This means calls like:
logger.BeginScope("string scope")
logger.BeginScope(new Dictionary<string, object>{"key", "value"}

do not contain any payload when listening to ActivityStart events.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Runtime untriaged New issue has not been triaged by the area owner labels Mar 11, 2020
@wiktork
Copy link
Member Author

wiktork commented Mar 11, 2020

Related pull requests:
aspnet/Logging/pull/424
dotnet/extensions/pull/870

@ericstj
Copy link
Member

ericstj commented Apr 3, 2020

/cc @noahfalk @maryamariyan

@maryamariyan maryamariyan removed the untriaged New issue has not been triaged by the area owner label Jun 22, 2020
@maryamariyan maryamariyan added this to the 5.0.0 milestone Jun 22, 2020
@maryamariyan
Copy link
Member

maryamariyan commented Jun 22, 2020

/cc: @tarekgh @shirhatti

@saurabh500
Copy link
Contributor

Wrong Saurabh ? :)

@maryamariyan
Copy link
Member

sorry @saurabh500 :) updated

@shirhatti
Copy link
Contributor

@wiktork Do we need this for dotnet-monitor?

@wiktork
Copy link
Member Author

wiktork commented Jun 23, 2020

@wiktork Do we need this for dotnet-monitor?

I wouldn't say need, more of a nice to have. It's a gap between inproc ILoggers and what we can collect.

@tarekgh tarekgh modified the milestones: 5.0.0, Future Jun 23, 2020
@maryamariyan
Copy link
Member

maryamariyan commented Oct 16, 2020

It's a gap between inproc ILoggers and what we can collect.

@wiktork could you please share a small code snippet to illustrate this limitation a bit better and showcase the difference in behavior you found?

@wiktork
Copy link
Member Author

wiktork commented Oct 16, 2020

We collect ActivityStart events that are emitted through EventSourceLogger:

https://github.com/dotnet/diagnostics/blob/b8f033696de1a1261040ac15273854cc42b8eddf/src/Microsoft.Diagnostics.Monitoring/DiagnosticsEventPipeProcessor.cs#L149-L182

However, EventSourceLogger only cares about object state that is of a specific type:

private IReadOnlyList<KeyValuePair<string, string>> GetProperties(object state)
{
if (state is IReadOnlyList<KeyValuePair<string, object>> keyValuePairs)
{

In the Asp.net app:

    public class HomeController : Controller
    {
        private readonly ILogger<HomeController> _logger;

        public HomeController(ILogger<HomeController> logger)
        {
            _logger = logger;
        }

        public IActionResult Index()
        {
            _logger.BeginScope(new Dictionary<string, object> { { "key", "value" } });
            _logger.BeginScope("some scope");

            return View();
        }

The scope data such as key/value and "some scope" are lost, since they are not IReadOnlyList<KeyValuePair<string, object>>. While it's probably not practical to serialize every object to json, at least dictionaries and strings should be supported.

Related to this issue is how the data is serialized:

writer.WriteStartObject();
foreach (KeyValuePair<string, string> keyValue in keyValues)
{
writer.WriteString(keyValue.Key, keyValue.Value);
}
writer.WriteEndObject();

Json supports more than just strings, but all the data becomes string values after serialization.

Please let me know if you need any more info.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants