Particular / NServiceBus

Build, version, and monitor better microservices with the most powerful service platform for .NET
https://particular.net/nservicebus/
Other
2.09k stars 648 forks source link

Introduce a NServiceBus.EndpointVersion header #1035

Closed andreasohlund closed 10 years ago

andreasohlund commented 11 years ago

To capture the version of the users endpoint that is running. Extract the current code (used in side by side mode in the host) and make it available on the core.

New api in v5

var configuration = new BusConfiguration();
configuration.EndpointVersion("1.2.1"))
johnsimons commented 11 years ago

@andreasohlund haven't u done this already?

andreasohlund commented 11 years ago

no

On Mon, May 6, 2013 at 7:20 AM, John Simons notifications@github.comwrote:

@andreasohlund https://github.com/andreasohlund haven't u done this already?

— Reply to this email directly or view it on GitHubhttps://github.com/NServiceBus/NServiceBus/issues/1035#issuecomment-17466725 .

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

SimonCropp commented 11 years ago

@andreasohlund why the Configure.Conventions in your sample? Wouldnt it be more like Configure.GetEndpointVersionAction =...

similar to how endpoint name is done

    public static Func<string> GetEndpointNameAction = () => EndpointHelper.GetDefaultEndpointName();
andreasohlund commented 11 years ago

I don't like it at the toplevel. Don't we have a Configure.Endpoint()?

Sent from my iPhone

On 26 maj 2013, at 10:24, Simon Cropp notifications@github.com wrote:

@andreasohlund why the Configure.Conventions in your sample? Wouldnt it be more like Configure.GetEndpointVersionAction =...

similar to how endpoint name is done

public static Func<string> GetEndpointNameAction = () => EndpointHelper.GetDefaultEndpointName();

— Reply to this email directly or view it on GitHub.

SimonCropp commented 11 years ago

@andreasohlund @johnsimons what is the point of EndpointConventions? https://github.com/NServiceBus/NServiceBus/blob/master/src/NServiceBus.Core/EndpointConventions.cs

SimonCropp commented 11 years ago

@andreasohlund @johnsimons so configuring this is made more difficult by fact that the version needs to be known before the configuration is loaded. We currently use the file version for this https://github.com/Particular/NServiceBus/blob/develop/src/hosting/NServiceBus.Hosting.Windows/EndpointType.cs#L50

So would a simplified config workaround be to allow some custom attribute to allow people to override the version?

SimonCropp commented 11 years ago

ping @jbogard‎

johnsimons commented 11 years ago

It seems that there are 2 issues here:

  1. The endpoint version
  2. The version of events

1. The endpoint version

This needs to be known by the host (at least our host does need it), so Configure.GetEndpointVersionAction doesn't work, we need something earlier in the pipeline. We could introduce a convention, something like class called Endpoint with method Foo!

2. The version of events

This needs to be extensible, so the users can control how to version events.

indualagarsamy commented 11 years ago

+1 on having the version of the message that is being handled as well. What is the purpose of the NServiceBus.Version in the headers? Does that one get replaced with the OriginatingEndpointVersion instead?

<HeaderInfo>
        <Key>NServiceBus.Version</Key>
        <Value>4.0.3</Value>
    </HeaderInfo>
andreasohlund commented 11 years ago

That is the version of nservicebus used at the sending endpoint. Endpoint version is the semver version of the users own code

On Mon, Aug 19, 2013 at 8:06 PM, Indu Alagarsamy notifications@github.comwrote:

What is the purpose of

NServiceBus.Version 4.0.3

— Reply to this email directly or view it on GitHubhttps://github.com/Particular/NServiceBus/issues/1035#issuecomment-22891837 .

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

indualagarsamy commented 11 years ago

So, can we have explicit names for the headers? i.e. OriginatingEndpointVersion and ReceivingEndpointVersion instead and get rid of the generic NServiceBus.Version header? Or am I completely missing the point here? :)

andreasohlund commented 11 years ago

I guess that would work, in essence we need 4 headers

SendingNSBVersion ReceivingNSBVersion

SendingEndpointVersion ReceivingEndpointVersion

On Mon, Aug 19, 2013 at 8:17 PM, Indu Alagarsamy notifications@github.comwrote:

So, can we have explicit names for the headers? i.e. OriginatingEndpointVersion and ReceivingEndpointVersion instead and get rid of the generic NServiceBus.Version header? Or am I completely missing the point here? :)

— Reply to this email directly or view it on GitHubhttps://github.com/Particular/NServiceBus/issues/1035#issuecomment-22892606 .

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

indualagarsamy commented 11 years ago

based on discussion with @andreasohlund

  1. The endpoint could just be a host where many dlls are deployed into it. In which case, will there be added value in getting the version of the message handler dll that is deployed into it, rather than just the endpoint version? So, while the endpoint host could be at version 1.0.1, it could have Sales.MessageHandler, which could be at 3.0.2 (which might be more interesting rather than just the host)
  2. Also about the actual message version, it might make sense to know both the version of the messages, the one that is sending/publishing and the one that is receiving. Knowing these will help in identifying deployment issues or lack thereof.. eg. Worker1 is still processing messages of schema version 1.0.1, whereas all the other workers have been upgraded to schema version 1.0.5, etc. This could be useful in doing impact analysis etc at a later point in time analyzing the data. Eg. What endpoints are running x version of the schema still, etc.
johnsimons commented 11 years ago

Headers overload!

Do we really need these many version headers? What is the use case for this?

johnsimons commented 11 years ago

@indualagarsamy the version is actually the messages dll version (if the user didn't override the convention)

dannycohen commented 11 years ago

Seems to me like the EndpointVersion you are proposing is an application specific version number (not directly related to the infrastructure). If so - why not leave it to the user to define and extend, as opposed to take ownership of it ?

andreasohlund commented 11 years ago

Yes, since this is likely to be related to ServiceControl we can provide a plugin that adds this (to allow Opie to see the version of his endpoints displayed in pulse)

On Tue, Aug 20, 2013 at 7:01 AM, Danny Cohen notifications@github.comwrote:

Seems to me like the EndpointVersion you are proposing is an application specific version number (not directly related to the infrastructure). If so - why not leave it to the user to define and extend, as opposed to take ownership of it ?

— Reply to this email directly or view it on GitHubhttps://github.com/Particular/NServiceBus/issues/1035#issuecomment-22922766 .

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

dannycohen commented 11 years ago

So your'e suggesting we allow adding headers as an extensibility feature (e.g. add a header) but not add it ourselves (and therefore - not take ownership of the specific implementation). Is this correct ?

andreasohlund commented 11 years ago

We already allow users to add their own headers. I was referring to us sayin "if you want to show detailed endpoint info including the version of your own business code in ServicePulse please drop this dll into the bin directory of your endpoints"?

On Tue, Aug 20, 2013 at 8:42 AM, Danny Cohen notifications@github.comwrote:

So your'e suggesting we allow adding headers as an extensibility feature (e.g. add a header) but not add it ourselves (and therefore - not take ownership of the specific implementation). Is this correct ?

— Reply to this email directly or view it on GitHubhttps://github.com/Particular/NServiceBus/issues/1035#issuecomment-22925538 .

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

dannycohen commented 11 years ago

Sounds good. Can this be achieved by having a user just mark a message property using an attribute (or a set of attributes) that will be translated later into ServiceInsight / ServicePulse data ? (e.g. aside from EndpointVersion, I'm thinking about "message title" that may replace the Message Id as the significant indicator of the message identity). Thoughts ?

andreasohlund commented 11 years ago

Yes that would be valuable when we add "business monitorig".Eg marking the "OrderValue" property with a special attribute (or more likely a convention) will promote it to a header so that we can give users "total order value per hour should be > x" type of monitoring

On Tue, Aug 20, 2013 at 8:54 AM, Danny Cohen notifications@github.comwrote:

Sounds good. Can this be achieved by having a user just mark a message property using an attribute (or a set of attributes) that will be translated later into ServiceInsight / ServicePulse data ? (e.g. aside from EndpointVersion, I'm thinking about "message title" that may replace the Message Id as the significant indicator of the message identity). Thoughts ?

— Reply to this email directly or view it on GitHubhttps://github.com/Particular/NServiceBus/issues/1035#issuecomment-22925942 .

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

SimonCropp commented 10 years ago

@andreasohlund please doco the new API inv5

andreasohlund commented 10 years ago

Api documented