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

Distributed tracing in networking #44063

Merged
merged 21 commits into from
Jan 29, 2025

Conversation

antonfirsov
Copy link
Member

@antonfirsov antonfirsov commented Dec 23, 2024

This is a major rework of #42830, extending the content, making the information more specific and IMO easier to follow. The biggest change is the in the collection paragraphs:

  • I dropped the definitions of terms instrumentation, collection, export, OpenTelemetry etc., given that we were already using them in the intro, so it felt rather pointless to define them at that point; IMO the reader is looking for practical guidelines and examples anyways.
  • Instead, I added a summary of the available methods, so readers can navigate the document following their preferences and goals.
  • Metrics.md: Added back the dotnet-monitor example. Reintroduced a short paragraph on prometheus/grafana, since the referenced docs need some augmentation.

Internal previews

📄 File 🔗 Preview link
docs/core/diagnostics/distributed-tracing-builtin-activities.md Built-in activities in .NET
docs/fundamentals/networking/telemetry/metrics.md Networking metrics
docs/fundamentals/networking/telemetry/overview.md Networking telemetry in .NET
docs/fundamentals/networking/telemetry/tracing.md docs/fundamentals/networking/telemetry/tracing
docs/fundamentals/toc.yml docs/fundamentals/toc

@antonfirsov antonfirsov modified the milestones: December 2024, April 2025 Dec 25, 2024
@antonfirsov antonfirsov marked this pull request as ready for review January 8, 2025 18:44
@antonfirsov antonfirsov requested a review from samsp-msft January 8, 2025 18:45
Copy link
Member

@samsp-msft samsp-msft left a comment

Choose a reason for hiding this comment

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

Great content. Lets make the changes to the core telemetry example so you don't need to document how to modify it.

docs/fundamentals/networking/telemetry/metrics.md Outdated Show resolved Hide resolved
docs/fundamentals/networking/telemetry/metrics.md Outdated Show resolved Hide resolved
docs/fundamentals/networking/telemetry/metrics.md Outdated Show resolved Hide resolved
docs/fundamentals/networking/telemetry/metrics.md Outdated Show resolved Hide resolved
docs/fundamentals/networking/telemetry/tracing.md Outdated Show resolved Hide resolved
docs/fundamentals/networking/telemetry/tracing.md Outdated Show resolved Hide resolved
docs/fundamentals/networking/telemetry/tracing.md Outdated Show resolved Hide resolved
docs/fundamentals/networking/telemetry/tracing.md Outdated Show resolved Hide resolved
docs/fundamentals/networking/telemetry/tracing.md Outdated Show resolved Hide resolved
@antonfirsov antonfirsov requested a review from gewarren January 23, 2025 18:25
@antonfirsov
Copy link
Member Author

@gewarren would you mind doing a language review on this? (Though I think it's mostly ok, so it's optional.)

Copy link
Contributor

@gewarren gewarren left a comment

Choose a reason for hiding this comment

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

I'll wait to review the rest until you've run Acrolinx (see one of the comments I left).

docs/fundamentals/networking/telemetry/metrics.md Outdated Show resolved Hide resolved
docs/fundamentals/networking/telemetry/metrics.md Outdated Show resolved Hide resolved
docs/fundamentals/networking/telemetry/metrics.md Outdated Show resolved Hide resolved
@antonfirsov
Copy link
Member Author

@gewarren I pushed the Acrolinx scores into the 80-90 zone in 17ea9e2.
There were many recommendations which were either very difficult to address or would end up in loss of nuanced information/context. I did not address those.

Copy link
Contributor

@gewarren gewarren left a comment

Choose a reason for hiding this comment

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

I didn't finish reviewing tracing.md, as it seems to duplicate a lot of the content in metrics.md. I'm guessing that's not intentional, so will hold off on reviewing the rest.

docs/fundamentals/networking/telemetry/overview.md Outdated Show resolved Hide resolved
docs/fundamentals/networking/telemetry/metrics.md Outdated Show resolved Hide resolved
docs/fundamentals/networking/telemetry/metrics.md Outdated Show resolved Hide resolved
docs/fundamentals/networking/telemetry/metrics.md Outdated Show resolved Hide resolved
docs/fundamentals/networking/telemetry/metrics.md Outdated Show resolved Hide resolved
docs/fundamentals/networking/telemetry/tracing.md Outdated Show resolved Hide resolved
docs/fundamentals/networking/telemetry/tracing.md Outdated Show resolved Hide resolved
docs/fundamentals/networking/telemetry/tracing.md Outdated Show resolved Hide resolved
docs/fundamentals/networking/telemetry/tracing.md Outdated Show resolved Hide resolved
docs/fundamentals/networking/telemetry/tracing.md Outdated Show resolved Hide resolved
docs/fundamentals/networking/telemetry/tracing.md Outdated Show resolved Hide resolved
docs/fundamentals/networking/telemetry/tracing.md Outdated Show resolved Hide resolved
docs/fundamentals/networking/telemetry/tracing.md Outdated Show resolved Hide resolved
docs/fundamentals/networking/telemetry/tracing.md Outdated Show resolved Hide resolved
docs/fundamentals/networking/telemetry/tracing.md Outdated Show resolved Hide resolved
docs/fundamentals/networking/telemetry/tracing.md Outdated Show resolved Hide resolved
docs/fundamentals/networking/telemetry/tracing.md Outdated Show resolved Hide resolved
docs/fundamentals/networking/telemetry/tracing.md Outdated Show resolved Hide resolved
docs/fundamentals/networking/telemetry/tracing.md Outdated Show resolved Hide resolved
@antonfirsov
Copy link
Member Author

@gewarren thanks a lot for chiming in, the content looks much better now!

@antonfirsov antonfirsov merged commit a1b948b into dotnet:main Jan 29, 2025
8 checks passed
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.

3 participants