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

Added telemetry injection point for Ask<T> #5297

Merged
merged 1 commit into from
Oct 8, 2021

Conversation

Aaronontheweb
Copy link
Member

Looking for a way to help trace timeouts inside Ask<T> operations - needed some way to tap into the TaskCompletionSource and to create an active ISpan before the operation begins.

Looking for a way to help trace timeouts inside `Ask<T>` operations - needed some way to tap into the `TaskCompletionSource` and to create an active `ISpan` before the operation begins.
@@ -1561,8 +1561,9 @@ public void StopSeedNodeProcess()
/// Received `Join` message and replies with `Welcome` message, containing
/// current gossip state, including the new joining member.
/// </summary>
/// <param name="node">TBD</param>
/// <param name="roles">TBD</param>
/// <param name="node">The unique address of the joining node.</param>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated XML-DOC fix I made to resolve a compiler warning.

/// <typeparam name="T">The type of output this <see cref="FutureActorRef{T}"/> expects.</typeparam>
/// <returns>A new, single-use <see cref="FutureActorRef{T}"/> instance.</returns>
[InternalApi]
FutureActorRef<T> CreateFutureRef<T>(TaskCompletionSource<T> tcs);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New InternalApi, but still public member on the IActorRefProvider interface. This is our injection point.

//create a new tempcontainer path
var path = provider.TempPath();

var future = new FutureActorRef<T>(result, path, provider);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved these two lines to the factory method - all of the TCS setup is left as-is.

Copy link
Contributor

@Arkatufus Arkatufus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Aaronontheweb
Copy link
Member Author

@Arkatufus thanks - still integration testing this with Phobos to make sure it actually works first.

@Aaronontheweb Aaronontheweb marked this pull request as ready for review October 8, 2021 02:20
@Aaronontheweb
Copy link
Member Author

Validated this on our end - looking good there.

@Aaronontheweb Aaronontheweb merged commit a091c2a into akkadotnet:dev Oct 8, 2021
@Aaronontheweb Aaronontheweb deleted the ask/factory-method branch October 8, 2021 02:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants