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

Expose an interface for DiagnosticContext collection flow #223

Open
julealgon opened this issue Dec 10, 2020 · 0 comments
Open

Expose an interface for DiagnosticContext collection flow #223

julealgon opened this issue Dec 10, 2020 · 0 comments

Comments

@julealgon
Copy link

Is your feature request related to a problem? Please describe.
I had a requirement where I needed to log multiple operations in a single event, but outside of AspNetCore. To make that work, I created a "wrapper" similar to the middleware in this library, relying on DiagnosticContext in the implementation, and on IDiagnosticContext in the consumer logging code.

However, I quickly realized that unit testing the behavior while using the concrete DiagnosticContext was very challenging. I was not able to force Set calls in the context while being able to persist the state in my unit test. I believe this is due to the AsyncLocal usage that happens internally, which disallows passing any information "from the outside" to the current context.

Describe the solution you'd like
I'd like either an interface that exposes the "collection side" of DiagnosticContext, so that I can rely on it instead of the concrete class in my implementation and be able to test or, alternatively, some sort of simpler implementation for the class tailored for testing, which would allow me to call Set from the outside and see those reflected in the context after BeginCollection is called.

I believe the former would be simpler to do, however.

Describe alternatives you've considered
I was unable to unit test the behavior around Set. I could've extracted an interface for that myself but decided against it for simplicity, then ended up testing the rest of the behavior without validating the added properties.

Additional context
I think IDiagnosticContext has broader usage than just AspNetCore, so perhaps it would be a good idea to move it away from this package and leave only the actual middleware here.

Additionally, I wish I actually didn't have to rely on IDiagnosticContext at all for the log aggregation behavior. More here (related) #222

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

1 participant