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

WIP : OpenTracing support #406

Merged
merged 12 commits into from Jun 15, 2018
Merged

WIP : OpenTracing support #406

merged 12 commits into from Jun 15, 2018

Conversation

jrouaix
Copy link
Contributor

@jrouaix jrouaix commented Jun 11, 2018

Chat example is instrumented with jager tracing.
You can start a jaeger instance with docker : https://www.jaegertracing.io/docs/getting-started/

image

@jrouaix jrouaix mentioned this pull request Jun 11, 2018
@rogeralsing
Copy link
Contributor

This is amazing! :-)
This will help so much for everyone building actor systems.

Does it also work locally on a single node, just to visualize actor interaction.

I also think, if we do this right, it could become extremely valuable for the cluster support.
To understand what is actually going on

@jrouaix
Copy link
Contributor Author

jrouaix commented Jun 12, 2018

Does it also work locally on a single node, just to visualize actor interaction.

It should, will confirm asap.

@jrouaix jrouaix changed the title OpenTracing support - Work in progress WIP : OpenTracing support - do not merge Jun 12, 2018
@rogeralsing
Copy link
Contributor

should we merge and work on details in another pr?

@jrouaix
Copy link
Contributor Author

jrouaix commented Jun 13, 2018

I have an nice example incomming. and a few more thoughts. Can you wait until tomorrow ?

@jrouaix
Copy link
Contributor Author

jrouaix commented Jun 14, 2018

From this interaction in mind I made this quick and dirty complex example.
image
Here is the result :
image

@jrouaix
Copy link
Contributor Author

jrouaix commented Jun 14, 2018

I'm not satisfied with the result, i'll try with decorators.

@rogeralsing rogeralsing mentioned this pull request Jun 14, 2018
@jrouaix
Copy link
Contributor Author

jrouaix commented Jun 14, 2018

Well I tried, but seems Props.ContextDecocator is not used, so I tried to implement the used ... and it's not that easy !

@rogeralsing
Copy link
Contributor

I think we need to commit on how decorators should work, should they be chainable like middleware, is it enough to just have a single wrapper over the actor context?

@rogeralsing
Copy link
Contributor

@jrouaix
Copy link
Contributor Author

jrouaix commented Jun 14, 2018

First results on decorator, now RequestAsync span is as long as the sum of all the process it will trigger
image

@jrouaix
Copy link
Contributor Author

jrouaix commented Jun 14, 2018

Will fix unit tests when api is more stable, now it's broken and it's a good reminder to udpate the tests.

@rogeralsing
Copy link
Contributor

Will fix unit tests when api is more stable, now it's broken and it's a good reminder to udpate the tests.

I didn't quite get this, is there anything I can/should do?

@jrouaix
Copy link
Contributor Author

jrouaix commented Jun 15, 2018

No i meant when the WithOpenTracing() stuffs are ok.
Do you have comments on what is done so far ?

@rogeralsing
Copy link
Contributor

No I think it all looks great,
There seems to be some compile error that needs fixing first though:

OpenTracingExtensionsTests.cs(32,56): error CS1501: No overload for method 'OpenTracingSenderMiddleware' takes 2 arguments [C:\projects\protoactor-dotnet\tests\Proto.OpenTracing.Tests\Proto.OpenTracing.Tests.csproj]
    31 Warning(s)
    2 Error(s)
Time Elapsed 00:02:09.73
An error occurred when executing task 'Build'.
Error: One or more errors occurred.
	.NET Core CLI: Process returned an error (exit code 1).

@jrouaix
Copy link
Contributor Author

jrouaix commented Jun 15, 2018

Yes, of course, will do soon.

@rogeralsing rogeralsing merged commit 79d67d3 into asynkron:dev Jun 15, 2018
@rogeralsing
Copy link
Contributor

I've merged it all in, but we need to fix those two failing tests for tracing

@jrouaix
Copy link
Contributor Author

jrouaix commented Jun 15, 2018

rogeralsing pushed a commit that referenced this pull request Sep 22, 2018
* Update build.cake

* implement stopping when max number of restarts exceeds allowed number in a period of time

* Removed _applySnapshot callback after persisting a snapshot.

* address issue (TryDequeue return True but get null result)

alexandrnikitin/MPMCQueue.NET#2

* No need to count messages

* Fix 2 csproj files causing Visual Studio for Mac not load bug.

* fix: utilize EventStream.Instance for DI example (#387)

* added ActorPropsRegistry.RegisterProps(Type, Func<Props, Props> (#378)

* Implement Forward.

* Fix csprojs.

* Reverse back RavenDB.

* Add ContextState.Stopped.

* Allow InvokeSystemMessageAsync in any state.

* Temp fix appveyor.yml cake version.

* .

* Send EndpointWatcher/EndpointWriter Stop message in Actor.

* Report error if receive user message after Actor already stooped.

* Fix test codes.

* Add StopAsync/Poison/PoisonAsync

* .

* Update dependencies

* Fix test codes.

* Update dependency for example/RemoteActivate.

* renamed RootContext => ActorClient. To be constitent with the class name.

* ActorClient.RequestAsync and example

* Moved Tell & Request from IContext to ISenderContext

* Mutable MessageEnvelope

* ISenderContext Message mutable

* static readonly mutable EmptyHeaders was really dangerous. Made it a new instance for each call.

* middlewares messages manipulation example

* Big Breaking Change

* tell -> send
* move send to extension
* revert mutating messages
* copyrights
* bugfix in actorcontext
* RootContext
* removed pid request
* cleanup router tests
* clean up tests
* cleanup
* more clean up
* even more
* immutable message headers
* better receive middleware
* looping spawn benchmark
* use props in actorcontext
* remove 1.5
* globaljson

* bugfix in rootcontext

* move Actor.Spawn to Context.Spawn

* Move props creating funcs from Actor to Props

* Code cleanup

* WIP : Single remote instance cluster- do not merge (#408)

* SingleRemoteInstanceProvider draft

* Should work but don't :-|

* working SingleRemoteInstanceProvider

* trimed csproj

* Docs

* formatting

* context proxy

* decorator context

* decorators works

* root context decorator

* fix IContext

* RootContext.With*

* WIP : OpenTracing support - do not merge (#406)

* OpenTracing first blood

* WithOpenTracingSenderTest

* oups

* OpenTracingReceiverMiddlewareTests

* jaeger working now ! (should I have read the manual sooner !)

* log messages

* ProtoTags targetPID & senderPID

* OpenTracing complex interactions example

* trying to play with decorator

* OpenTracing improvements with decorators

* fix non compiling tests

* remove globaljson

* Opentracing improvements (#412)

* OpenTracing first blood

* WithOpenTracingSenderTest

* oups

* OpenTracingReceiverMiddlewareTests

* jaeger working now ! (should I have read the manual sooner !)

* log messages

* ProtoTags targetPID & senderPID

* OpenTracing complex interactions example

* trying to play with decorator

* OpenTracing improvements with decorators

* OpenTracing improvements

* moved Proto.OpenTracing.Tests to "test" solution folder

* tried to have OpenTracing receive in ContextDecorator but failed

* OpenTracing back on tracks with middlewares

* unit tests unbroked and improved

* ensure decorator.Receive is called

* fix context decorator for actors without receive middleware

* finally receive decorator is working fine, but only triggered when at least one receive middleware (#414)

* Chainable context decorators (#417)

* Life cycle events : write deadletters, just in case

* OpenTracing commented code out

* Chainable context decorators

* Fixed issue with namespaces with deeper nesting (#419)

* Bump to 0.2.0

* Copy GuardianStrategy when copy Props.

* targeting netstandard2.0 and net452
@jrouaix jrouaix changed the title WIP : OpenTracing support - do not merge WIP : OpenTracing support Feb 20, 2020
rogeralsing added a commit that referenced this pull request Mar 6, 2020
* Release 0.2.0 (#421)

* Update build.cake

* implement stopping when max number of restarts exceeds allowed number in a period of time

* Removed _applySnapshot callback after persisting a snapshot.

* address issue (TryDequeue return True but get null result)

alexandrnikitin/MPMCQueue.NET#2

* No need to count messages

* Fix 2 csproj files causing Visual Studio for Mac not load bug.

* fix: utilize EventStream.Instance for DI example (#387)

* added ActorPropsRegistry.RegisterProps(Type, Func<Props, Props> (#378)

* Implement Forward.

* Fix csprojs.

* Reverse back RavenDB.

* Add ContextState.Stopped.

* Allow InvokeSystemMessageAsync in any state.

* Temp fix appveyor.yml cake version.

* .

* Send EndpointWatcher/EndpointWriter Stop message in Actor.

* Report error if receive user message after Actor already stooped.

* Fix test codes.

* Add StopAsync/Poison/PoisonAsync

* .

* Update dependencies

* Fix test codes.

* Update dependency for example/RemoteActivate.

* renamed RootContext => ActorClient. To be constitent with the class name.

* ActorClient.RequestAsync and example

* Moved Tell & Request from IContext to ISenderContext

* Mutable MessageEnvelope

* ISenderContext Message mutable

* static readonly mutable EmptyHeaders was really dangerous. Made it a new instance for each call.

* middlewares messages manipulation example

* Big Breaking Change

* tell -> send
* move send to extension
* revert mutating messages
* copyrights
* bugfix in actorcontext
* RootContext
* removed pid request
* cleanup router tests
* clean up tests
* cleanup
* more clean up
* even more
* immutable message headers
* better receive middleware
* looping spawn benchmark
* use props in actorcontext
* remove 1.5
* globaljson

* bugfix in rootcontext

* move Actor.Spawn to Context.Spawn

* Move props creating funcs from Actor to Props

* Code cleanup

* WIP : Single remote instance cluster- do not merge (#408)

* SingleRemoteInstanceProvider draft

* Should work but don't :-|

* working SingleRemoteInstanceProvider

* trimed csproj

* Docs

* formatting

* context proxy

* decorator context

* decorators works

* root context decorator

* fix IContext

* RootContext.With*

* WIP : OpenTracing support - do not merge (#406)

* OpenTracing first blood

* WithOpenTracingSenderTest

* oups

* OpenTracingReceiverMiddlewareTests

* jaeger working now ! (should I have read the manual sooner !)

* log messages

* ProtoTags targetPID & senderPID

* OpenTracing complex interactions example

* trying to play with decorator

* OpenTracing improvements with decorators

* fix non compiling tests

* remove globaljson

* Opentracing improvements (#412)

* OpenTracing first blood

* WithOpenTracingSenderTest

* oups

* OpenTracingReceiverMiddlewareTests

* jaeger working now ! (should I have read the manual sooner !)

* log messages

* ProtoTags targetPID & senderPID

* OpenTracing complex interactions example

* trying to play with decorator

* OpenTracing improvements with decorators

* OpenTracing improvements

* moved Proto.OpenTracing.Tests to "test" solution folder

* tried to have OpenTracing receive in ContextDecorator but failed

* OpenTracing back on tracks with middlewares

* unit tests unbroked and improved

* ensure decorator.Receive is called

* fix context decorator for actors without receive middleware

* finally receive decorator is working fine, but only triggered when at least one receive middleware (#414)

* Chainable context decorators (#417)

* Life cycle events : write deadletters, just in case

* OpenTracing commented code out

* Chainable context decorators

* Fixed issue with namespaces with deeper nesting (#419)

* Bump to 0.2.0

* Copy GuardianStrategy when copy Props.

* targeting netstandard2.0 and net452

* Upgrading code, packages, a bit of cleanup

* Merge, cleaning up mongo and logs

* Update Appveyor image

* Fixing the remote manager

* Fix target framework

* Fixes #403 (I hope)

Co-authored-by: Roger Johansson <rogeralsing@gmail.com>
Co-authored-by: Christian Palmstierna <christian.palmstierna@gmail.com>
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 this pull request may close these issues.

None yet

2 participants