Skip to content

EventSource improvements for text logging #12730

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

Closed
noahfalk opened this issue May 21, 2019 · 11 comments
Closed

EventSource improvements for text logging #12730

noahfalk opened this issue May 21, 2019 · 11 comments

Comments

@noahfalk
Copy link
Member

From an email thread with Krzysztof Cwalina and Pavel Krymets we were discussing that EventSource has rough edges when you try to use it for textual logging:

This is pretty much all I would have, built-in sinks, built-in simple text source, support for Azure functions (whether it needs multitenancy or not).

One other issue I’ve seen is a performance of in-process EventSource listener, there are a lot of allocations per event.

@pakrym
Copy link
Contributor

pakrym commented May 21, 2019

  • No good way to log Exception objects directly

@pakrym
Copy link
Contributor

pakrym commented May 23, 2019

  • Limited size of payload that forces us to roll out our own payload splitting mechanism

@noahfalk
Copy link
Member Author

What scale of payload do you need to log?

@noahfalk
Copy link
Member Author

For exceptions do you have any particular expectations for what the serialized format is?

@pakrym
Copy link
Contributor

pakrym commented May 24, 2019

For in-process listeners, I'd like to be able to pass Exception instance and strings of arbitrary length around. The problem is that it doesn't work with ETW.

@noahfalk
Copy link
Member Author

The context of this issue is that EventSource events were getting serialized as a string so they could be put into a text log. That requires the runtime to convert the exception into a string of some form. If you want to pass around exception object instances and get references back as-is from an in-proc sink that sounds like a scenario DiagnosticsSource was aiming to handle. If you also didn't want to use DiagnosticsSource for whatever reason but you did want the sink to receive an instance of arbitrary Exceptions then we should probably revisit the overall scenario because that wasn't the trajectory I thought we were on.

@pakrym
Copy link
Contributor

pakrym commented May 24, 2019

Let me clarify where I'm coming from. Even if we add support for console text logging to EventSource we would still need a way to forward events from EventSource to ILogger for scenarios where the library is used in ASP.NET Core environment.

So you are right that this is not a request for purely "text logging" improvement, more something we would need to have if EventSource would be the only logging mechanism used by the library.

@noahfalk
Copy link
Member Author

Gotcha - I just opened dotnet/coreclr#24773 which I think is a little more targeted at your aim.

@davidfowl
Copy link
Member

I got another one to add about object lifetime, thread safety and testability. I've spent the last day fighting with EventSource and EventListener trying to write a set of unit tests that run in parallel. Even when creating objects that look like they don't mutate global state, it seems like they still do. I think it's a massive design flaw of the current object model to do things implicitly like that.

@ghost
Copy link

ghost commented Dec 5, 2023

Due to lack of recent activity, this issue has been marked as a candidate for backlog cleanup. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will undo this process.

This process is part of our issue cleanup automation.

@ghost ghost added backlog-cleanup-candidate An inactive issue that has been marked for automated closure. no-recent-activity labels Dec 5, 2023
@ghost
Copy link

ghost commented Dec 19, 2023

This issue will now be closed since it had been marked no-recent-activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the issue, but please note that the issue will be locked if it remains inactive for another 30 days.

@ghost ghost closed this as completed Dec 19, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jan 18, 2024
@ghost ghost removed no-recent-activity backlog-cleanup-candidate An inactive issue that has been marked for automated closure. labels Jan 18, 2024
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants