Skip to content

Update manual_instrumentation.md adding .Net entry #27424

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

Merged
merged 22 commits into from
Feb 7, 2025

Conversation

gitstevenpham
Copy link
Contributor

@gitstevenpham gitstevenpham commented Feb 3, 2025

What does this PR do? What is the motivation?

Adds .Net entry based on https://dd.slack.com/archives/C05C61J554L/p1738011042428139

Merge instructions

Merge readiness:

  • Ready for merge

Merge queue is enabled in this repo. To have it automatically merged after it receives the required reviews, create the PR (from a branch that follows the <yourname>/description naming convention) and then add the following PR comment:

/merge

Additional notes

@gitstevenpham gitstevenpham requested review from a team as code owners February 3, 2025 18:57
Copy link
Contributor

github-actions bot commented Feb 3, 2025

Preview links (active after the build_preview check completes)

Modified Files

@gitstevenpham gitstevenpham force-pushed the steven.pham/dotnet-entry-manual-instrumentation branch from 70ff78c to 6aa7930 Compare February 3, 2025 19:21
remove extra `{{% /tab %}}`
Copy link
Contributor

@vandonr vandonr left a comment

Choose a reason for hiding this comment

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

maybe we want to mention that doing that will also propagate the trace context ?
We could also link to the doc explaining how to trace propagate without DSM, which also has some nice examples of header getter/setter methods for some queues.

I'd even link in the other direction, from that doc linked above to here, like "hey, I see you want to inject trace context, how about injecting DSM at the same time ?"

{{% /tab %}}
{{% tab ".Net" %}}
<div class="alert alert-warning">
<strong>Note:</strong> DSM instrumentation does not work in Async operations
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this wording is not too strong... It will work, it's just that context is broken/lost on a send following a receive. I don't know how to convey that in a clear way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How does:

Note: In async operations, this may not work as expected because the context derived from the incoming message can be lost when producing a new message in different threads.

Sound?

@gitstevenpham
Copy link
Contributor Author

gitstevenpham commented Feb 3, 2025

maybe we want to mention that doing that will also propagate the trace context ? We could also link to the doc explaining how to trace propagate without DSM, which also has some nice examples of header getter/setter methods for some queues.

I'd even link in the other direction, from that doc linked above to here, like "hey, I see you want to inject trace context, how about injecting DSM at the same time ?"

Added "The following example propogates the trace context. See Trace Context Propagation on how this is done in general."

Copy link
Contributor

@buraizu buraizu left a comment

Choose a reason for hiding this comment

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

Requesting some minor edits

@gitstevenpham
Copy link
Contributor Author

@buraizu can you help with the linking. I can't seem to get it to work

Copy link
Contributor

@johannbotha johannbotha left a comment

Choose a reason for hiding this comment

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

LGTM

@gitstevenpham
Copy link
Contributor Author

/merge

@dd-devflow
Copy link

dd-devflow bot commented Feb 7, 2025

Devflow running: /merge

View all feedbacks in Devflow UI.


2025-02-07 15:56:48 UTC ℹ️ MergeQueue: pull request added to the queue

The median merge time in master is 7m.


2025-02-07 16:04:54 UTC ℹ️ MergeQueue: This merge request was merged

@dd-mergequeue dd-mergequeue bot merged commit 5c2d05a into master Feb 7, 2025
18 of 20 checks passed
@dd-mergequeue dd-mergequeue bot deleted the steven.pham/dotnet-entry-manual-instrumentation branch February 7, 2025 16:04
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.

4 participants