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

Introduce disable audit #1309

Closed
3 of 4 tasks
johnsimons opened this issue Jul 15, 2013 · 7 comments · Fixed by #1399
Closed
3 of 4 tasks

Introduce disable audit #1309

johnsimons opened this issue Jul 15, 2013 · 7 comments · Fixed by #1399
Milestone

Comments

@johnsimons
Copy link
Member

Background

In v4 we changed the audit feature so that it can be configured via the registry, this has the advantage of centralized administration of audit queue via group policy.

However this has also introduced a lot of friction to turn off audit per endpoint.

At the moment if the config file does not have ForwardReceivedMessagesTo attribute we then check the registry HKEY_LOCAL_MACHINE\SOFTWARE\ParticularSoftware\ServiceBus\[AuditQueue] and only then we disable the audit feature.

So the above workflow has a few disadvantages:

  • User needs to check registry and delete keys to turn off audit;
  • Not possible to have EndpointA with centralized audit managed by registry and EndpointB with audit turned off;
  • In a dev box with registry key, audit works but once user migrated to PROD machine audit may not work if user does not have reg key set!

Proposed Solution

  • Make Audit a feature
  • Introduce an explicit code only API to turn off audit on the endpoint, eg:
    .Configure.Features.Disable<Audit>();
  • Throw an exception (so that endpoint terminates) at initialization if ForwardReceivedMessagesTo attribute missing and no registry key set.
    The exception text should explain that they either have to call .Configure.Features.Disable<Audit>(); or use Set-NServiceBusLocalMachineSettings cmdlet.
  • Log a warning in debug mode only if ForwardReceivedMessagesTo attribute missing but registry key is set, here is a start:
Endpoint audit is configured using the registry, please ensure that you run Set-NServiceBusLocalMachineSettings cmdlet on the target deployment machine.
@indualagarsamy
Copy link
Contributor

@johnsimons - why

Configure.Endpoint.Advanced(s => e.DisableAudit()); 

and why not

Configure.Features.Disable<Audit>()

@johnsimons
Copy link
Member Author

I think you mean:

.Configure.Features.Disable<Audit>();

I like it 👍
But this means that we would need to make the Audit a feature.

@andreasohlund thoughts?

@andreasohlund
Copy link
Member

I like it

Sent from my iPhone

On 18 jul 2013, at 06:49, John Simons notifications@github.com wrote:

I think you mean:

.Configure.Features.Disable();
I like it
But this means that we would need to make the Audit a feature.

@andreasohlund thoughts?


Reply to this email directly or view it on GitHub.

@indualagarsamy
Copy link
Contributor

About this: "Throw an exception (so that endpoint terminates) at initialization if ForwardReceivedMessagesTo attribute missing and no registry key set. The exception text should explain that they either have to call .Configure.Features.Disable<Audit>(); or use Set-NServiceBusLocalMachineSettings cmdlet.

Current behavior: Auditing is off when the registry keys are not present & the forward attributes in the UnicastBusConfig is not present. While the nuget packages now add the config section, the admin can turn off auditing by removing the attribute in the config and clearing entries in registry.

With the proposed change, is a breaking change: This means that in turning auditing off is no longer the administrator concern but the programmer's. As we now need .Configure.Features.Disable<Audit>(); in the initialization code. Is this right?

cc @udidahan @andreasohlund

@andreasohlund
Copy link
Member

I think audit on/off should be in the hands of the admins so I'd vote to turn it off if no config is present. Ie the same behaviour as in 4.0

Sent from my iPhone

On 6 aug 2013, at 21:59, Indu Alagarsamy notifications@github.com wrote:

About this: "Throw an exception (so that endpoint terminates) at initialization if ForwardReceivedMessagesTo attribute missing and no registry key set. The exception text should explain that they either have to call .Configure.Features.Disable(); or use Set-NServiceBusLocalMachineSettings cmdlet.

Current behavior: Auditing is off when the registry keys are not present & the forward attributes in the UnicastBusConfig is not present. While the nuget packages now add the config section, the admin can turn off auditing by removing the attribute in the config and clearing entries in registry.

With the proposed change, is a breaking change: This means that in turning auditing off is no longer the administrator concern but the programmer's. As we now need .Configure.Features.Disable(); in the initialization code. Is this right?

cc @udidahan @andreasohlund


Reply to this email directly or view it on GitHub.

@udidahan
Copy link
Member

udidahan commented Aug 6, 2013

We don’t want to people to accidentally disable auditing – it’s not an admin-only consideration.

@andreasohlund
Copy link
Member

I see your point Udi, lets go ahead as planned Indu

On Tue, Aug 6, 2013 at 10:38 PM, Udi Dahan notifications@github.com wrote:

We don’t want to people to accidentally disable auditing – it’s not an
admin-only consideration.

From: Andreas Öhlund [mailto:notifications@github.com]
Sent: Tuesday, August 06, 2013 11:24 PM
To: NServiceBus/NServiceBus
Cc: Udi Dahan
Subject: Re: [NServiceBus] Introduce disable audit (#1309)

I think audit on/off should be in the hands of the admins so I'd vote to
turn it off if no config is present. Ie the same behaviour as in 4.0

Sent from my iPhone

On 6 aug 2013, at 21:59, Indu Alagarsamy notifications@github.com
wrote:

About this: "Throw an exception (so that endpoint terminates) at
initialization if ForwardReceivedMessagesTo attribute missing and no
registry key set. The exception text should explain that they either have
to call .Configure.Features.Disable(); or use
Set-NServiceBusLocalMachineSettings cmdlet.

Current behavior: Auditing is off when the registry keys are not present
& the forward attributes in the UnicastBusConfig is not present. While the
nuget packages now add the config section, the admin can turn off auditing
by removing the attribute in the config and clearing entries in registry.

With the proposed change, is a breaking change: This means that in
turning auditing off is no longer the administrator concern but the
programmer's. As we now need .Configure.Features.Disable(); in the
initialization code. Is this right?

cc @udidahan @andreasohlund


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub <
https://github.com/NServiceBus/NServiceBus/issues/1309#issuecomment-22208297>
.Image removed by sender.


No virus found in this message.
Checked by AVG - www.avg.com
Version: 2013.0.3392 / Virus Database: 3209/6545 - Release Date: 08/02/13


Reply to this email directly or view it on GitHubhttps://github.com//issues/1309#issuecomment-22209377
.

http://andreasohlund.net
http://twitter.com/andreasohlund

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

Successfully merging a pull request may close this issue.

4 participants