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

Add support for multiple implementations of IEventSink #1147

Closed
krxprn opened this issue Mar 1, 2024 · 5 comments
Closed

Add support for multiple implementations of IEventSink #1147

krxprn opened this issue Mar 1, 2024 · 5 comments

Comments

@krxprn
Copy link

krxprn commented Mar 1, 2024

Which version of Duende IdentityServer are you using?
6

Which version of .NET are you using?
6

Describe the bug
I need to be able to register multiple implementations of IEventSink

A clear and concise description of what the bug is.

To Reproduce
Currently we're only able to create one huge class for different sinks which is far from ideal

Steps to reproduce the behavior.

Expected behavior

A clear and concise description of what you expected to happen.

Log output/exception with stacktrace
I expect to be able to
ToServiceBus : IEventSink
ToFile : IEventSink
ToSerilog : IEventSink

data

Additional context

This was requested a while back and taken into consideration, but ignored after?
IdentityServer/IdentityServer4#3771

@AndersAbel
Copy link
Member

.NET 8 and IdentityServer v7 has much improved support for Open Telemetry. We think that Open Telemetry in most cases are a better path forward than the events. We are even considering if we should remove the events in a future release in favour of Open Telemetry.

Could you please tell us a bit more about how you use the events? If you've had a chance to look at the Open Telemetry support, is there anything you need that isn't handled by Open Telemetry?

@krxprn
Copy link
Author

krxprn commented Mar 7, 2024

Hi AndersAbel,

A couple of use cases:

  1. ClientLastAccessedSink : IEventSink
  2. AuditSink : IEventSink

We're currently licensed for IdentiyServer v6 - unsure about future plans to update that to v7; .NET 8 upgrades have not yet commenced but it's good to have in mind, thank you!

@AndersAbel
Copy link
Member

Thanks for getting back. If I guess right from the names these are not for technical logging but rather for audit logging?

While we see that Open Telemetry is the path forward for technical logging/instrumentation we are still investigating how audit logging fits into this. Tentatively audit logging will not be a good fit for Open Telemetry but will rather require something else, like the existing eventing model.

Except for the request for allowing multiple sinks, is the event system working well for your requirements?

@krxprn
Copy link
Author

krxprn commented Mar 11, 2024

Hi Anders,

This is correct, these are not technical logging bits but audit logging ones.

Yes the event system currently suffice our requirements, the events provide enough of meta data for business logic decisions to be made upon. We currently use couple of channels for event logging - SQL updating Identity Server's client table LastAccessed date, which is really important and would be a driver for business decisions. We also use EventHubs for internal auditing.

As initially stated we'd like to have separate implementations for that instead of bloated logic in a single method.

I hope the above helps! Have a good week ahead.

@AndersAbel
Copy link
Member

Thank you for your feedback. I've opened an implementation issue about allowing multiple IEventSink registrations: DuendeSoftware/IdentityServer#1531.

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

2 participants