-
Notifications
You must be signed in to change notification settings - Fork 780
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
[docs] ASP.NET Core tracing get started doc #4076
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #4076 +/- ##
=======================================
Coverage 85.61% 85.62%
=======================================
Files 289 289
Lines 11255 11255
=======================================
+ Hits 9636 9637 +1
+ Misses 1619 1618 -1
|
Notes to be discussed: .NET's own logging doc splits the logging into 2 - host based vs non-host based, with an entirely separate doc for asp.net core |
Create a new web application: | ||
|
||
```sh | ||
dotnet new webapp -o aspnetcoreapp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets do webapi so that the # of files to be checked in are minimal, and also gets rid of the other funny spellcheck thing you mentioned as well!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not opposed to that. FYI the reason I went with the razor web app style is when I searched "aspnetcore get started" I landed on Tutorial: Get started with ASP.NET Core. Since it does have a UI, I also added this:
Makes it kind of fun.
But agree it sucks to have all of those files in there. I'm going to mess with it a bit and see if they can be pulled at build time.
```sh | ||
dotnet add package OpenTelemetry.Exporter.Console --prerelease | ||
dotnet add package OpenTelemetry.Extensions.Hosting --prerelease | ||
dotnet add package OpenTelemetry.Instrumentation.AspNetCore --prerelease |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am neutral here... The getting started could be strictly "how to manually create a span in 5 minutes". A "next step" could show the instrumentation library stuff, instead of manual span creation.
Showing instrumentation library here will truly make it a 5 min tutorial and is the likely path a normal user would take.
Will discuss in SIG meetings to get more views on this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Showing instrumentation library here will truly make it a 5 min tutorial and is the likely path a normal user would take.
Sort of what I was thinking. Get up and running quickly, see some data for incoming requests. Manually creating spans could be a next topic/advanced doc for users who are interested 🤷♀️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Manually creating spans could be a next topic/advanced doc
+1. Yes, I've always thought of manually generating telemetry as an advanced topic not a step 1 topic. A lot of folk are coming to OpenTelemetry having used other proprietary products many of which just require a simple install/configuration to see some magic.
https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/examples/Console - This is a prime candidate for nuking. This was meant for developers mainly, not really written with end users in mind. Depending on how this PR goes, we can tweak the examples folder. |
I would vote for not covering ASP.NET for now since it is considered "legacy", if it turned out that there is a high demand (I guess very unlikely), we can add it later. |
Ya we could go a lot of ways with it. I think "Host & Non-host" would be good, but might not resonate with users the same way "ASP.NET Core & Console" would. Truth be told the "Console" example is lying because you can make a console app with generic host and use the hosting extensions 😄
Maybe this is where the example apps would be useful. We could just link to them for some other scenarios? For example in contrib we have ASP.NET example. |
I've had similar issues in other repos. I addressed this by removing all the js and css files. In my opinion the Error and Privacy pages can also be removed. |
I switched all the npm stuff to use a CDN for retrieval so the artifacts won't be in our repo. The misspell warning should clear but there seems to be an issue with the job at the moment 🤕
I don't feel passionately about this but if we do stick with the razor page style I prefer it to render somewhat nicely & be close to what the user will see with the command line template if they follow the instructions in the README. |
+1 Maybe here in the For the "more real world examples", combine the effort with opentelemetry-demo project and put the code there? |
Updated to use |
@@ -41,7 +41,8 @@ If you are new here, please read the getting started docs: | |||
|
|||
* [logs](./docs/logs/getting-started/README.md) | |||
* [metrics](./docs/metrics/getting-started/README.md) | |||
* [traces](./docs/trace/getting-started/README.md) | |||
* traces: [ASP.NET Core](./docs/trace/getting-started-aspnetcore/README.md) | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I suggest keep this unchanged, by default users will go to the simple console example (assuming a new user just came and wanted to understand the basics, and the user doesn't have to mix this with the actual integration of OpenTelemetry and an application framework). In the console tutorial doc, I think we can add something in the beginning "this is showing the basics of a simple console application, if you want to learn how to use OpenTelemetry with ASP.NET Core, read X, if you want to learn how to use UWP, read Y, ..."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we do the reverse? Link to the ASP.NET Core case with a comment for simple console/.NET Framework? I'm assuming more people will be interested in the ASP.NET Core style.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know, here is one thing we need to consider: for folks who want to learn how to extend the SDK or how to configure advanced scenarios, do we want them to get into ASP.NET Core or stay with a basic console app? Or we can ask the question in another way - would C# "Hello, World" choose console or ASP.NET Core?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I'm fine with the "getting started with tracing" doc pointing to ASP.NET Core demo, and in the banner put something like "hey if you don't want to bother with HTTP hosting, here goes a simple console version you want to use" (it seems a bit awkward to me, but I don't see big no).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for folks who want to learn how to extend the SDK or how to configure advanced scenarios, do we want them to get into ASP.NET Core or stay with a basic console app?
I think this is a good question, and maybe it's true that if a custom SDK extension is your goal then you might want a guide that used a basic console app. Though, I'd say that if you're writing a custom SDK extension, then you're probably already past "getting started" with OTel 😆.
I think our "getting started" users are most likely folks who just want to see some telemetry get emitted from an app, and personally I think the fact we're entangling our getting started guide with an application framework actually serves to better meet this need - i.e., as a new user I don't need to entangle myself with learning anything about the activity api out of the gate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fwiw here is what I did in the .NET docs: https://learn.microsoft.com/en-us/dotnet/core/diagnostics/distributed-tracing-collection-walkthroughs#collect-traces-using-opentelemetry
I did ASP.NET first, console 2nd. My assumption is that ASP.NET is the more common case users will care about and it also has the advantage that you don't need to add any manual instrumentation to see a result for distributed tracing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I chatted with @cijothomas a bit offline. I think ideally we would only have the ASP.NET Core guide. Agree with @noahfalk it seems like what most users will be after and is nice and concise. Manual instrumentation would be a more advanced topic/guide. Console example might be in some other repo. However, because the ASP.NET Core guide currently relies on prerelease packages, and that doesn't seem likely to change anytime soon, we feel like having the console guide close by will be useful for some teams. We may have to live with the two for now. But I did add a banner on the ASP.NET Core guide with a "note" about the prerelease status.
The only open question for me is do we link to both from the main README? Don't have a strong opinion either way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No strong opinion either (slightly prefer having only one link instead of two, to keep the doc smaller).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a strong opinion, but its not just the application type that is changing between 2 tutorials - it also changes manual span creation <-> instrumentation library creation. I do not have a neat alternative suggestion either, lets proceed and keep iterating if fresh ideas come up later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Changes
Received some feedback from @noahfalk that the getting started docs for tracing didn't really point towards the new "StartWithHost" pattern. I tried to fix this up by adding a dedicated ASP.NET Core guide and tweaking the text going over
TracerProvider
/TracerProviderBuilder
to show both patterns.