Skip to content

Conversation

jaycdave88
Copy link
Contributor

@jaycdave88 jaycdave88 commented Jul 22, 2021

What does this PR do?

Updated the .NET Core & .NET Framework Compatibility docs.

Motivation

Lacking information on new integrations and needed a refresh on the data.

Preview

https://docs-staging.datadoghq.com/jay.dave/dotnet_new_integrations/tracing/setup_overview/compatibility_requirements/dotnet-framework/
https://docs-staging.datadoghq.com/jay.dave/dotnet_new_integrations/tracing/setup_overview/compatibility_requirements/dotnet-core/

https://docs-staging.datadoghq.com/jay.dave/dotnet_new_integrations/tracing/setup_overview/setup/dotnet-framework/?tab=environmentvariables#capabilities
https://docs-staging.datadoghq.com/jay.dave/dotnet_new_integrations/tracing/setup_overview/setup/dotnet-core?tab=windows#capabilities

Additional Notes

This is ready to go!


Reviewer checklist

  • Review the changed files.
  • Review the URLs listed in the Preview section.
  • Check images for PII
  • Review any mentions of "Contact Datadog support" for internal support documentation.

@jaycdave88 jaycdave88 requested a review from a team as a code owner July 22, 2021 23:32
@github-actions github-actions bot added the tracing Content changed in the tracing folder label Jul 22, 2021
@jaycdave88 jaycdave88 added the Do Not Merge Just do not merge this PR :) label Jul 23, 2021
Copy link
Member

@andrewlock andrewlock left a comment

Choose a reason for hiding this comment

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

Looks good, though I wonder whether some of the content belongs here. Also there's a couple of comments from bots about capitalisation

@jaycdave88 jaycdave88 requested a review from andrewlock July 23, 2021 21:39
jaycdave88 and others added 2 commits July 27, 2021 13:24
…otnet-core.md

Co-authored-by: Lucas Pimentel-Ordyna <lucas.pimentel@datadoghq.com>
…otnet-core.md

Co-authored-by: Lucas Pimentel-Ordyna <lucas.pimentel@datadoghq.com>
@jaycdave88 jaycdave88 requested a review from lucaspimentel July 27, 2021 20:26
…otnet-core.md

Co-authored-by: Lucas Pimentel-Ordyna <lucas.pimentel@datadoghq.com>
@jaycdave88 jaycdave88 requested a review from lucaspimentel July 27, 2021 21:45
@jaycdave88 jaycdave88 removed the Do Not Merge Just do not merge this PR :) label Jul 28, 2021
@lucaspimentel
Copy link
Member

The previews haven't updated in a while 😞

jaycdave88 and others added 3 commits July 28, 2021 11:20
@jaycdave88 jaycdave88 requested a review from lucaspimentel July 28, 2021 18:51
@@ -47,7 +47,10 @@ further_reading:
---
## Compatibility requirements

The .NET Tracer supports automatic instrumentation on .NET Framework 4.5 and above. For a full list of supported libraries, visit the [Compatibility Requirements][1] page.
### Supported .NET Framework runtimes
The .NET Tracer supports instrumentation on .NET Framework 4.5 and above (using CLR v4.0).
Copy link
Member

Choose a reason for hiding this comment

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

I think many users will find "using CLR v4.0" confusing and it's not very helpful in this context. I would remove this from this PR and (in a separate PR if you want, doesn't need to block this PR) add it to the IIS section below and explain how it applies per IIS Application Pool. When we do that, we should also mention whether we support "Classic" pipeline mode or not.

image

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 to removing the "using CLR v4.0" text from this section for this PR

Copy link
Contributor Author

@jaycdave88 jaycdave88 Aug 11, 2021

Choose a reason for hiding this comment

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

ACK - will remove "using CLR v4.0" from the section and leave it as follows:

The .NET Tracer supports instrumentation on .NET Framework 4.5 and above.

For a full list of supported libraries and processor architectures, see Compatibility Requirements.

Comment on lines 44 to 48
| Version |
| -------------------- |
| .NET 5 |
| .NET Core 3.1 |
| .NET Core 2.1 |
Copy link
Member

Choose a reason for hiding this comment

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

We removed this table on the .NET Framework side, but it's still here in .NET Core. It takes up more space and pushes "Installation and getting started" under the fold. This is not great considering that this is the "setup overview" page and we have an separate "Compatibility" page for this information.

Also, using tables for non-tabular data is not good semantic html.

image

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ACK - I have updated to reflect:

The .NET Tracer supports instrumentation on .NET Core 2.1, 3.1, and .NET 5.

For a full list of supported libraries and processor architectures, see Compatibility Requirements.

- Linux x64 (`linux-x64`)
- Alpine Linux x64 (`linux-musl-x64`)
- Linux ARM64 (`linux-arm64`) Added in version 1.27.0, automatic instrumentation only supported on .NET 5.
### Supported .NET Core runtimes
Copy link
Member

Choose a reason for hiding this comment

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

[applies to both .NET Core and .NET Framework]

If we're adding another navigation level for "Supported .NET Core runtimes" and "Supported Processor Architectures", we can probably remove the top-level "Compatibility" since it's now become redundant.

The outline is now:

  • [Page title] .NET Core Compatibility Requirements
    • Compatibility (is this adding anything useful?)
      • Supported .NET Core runtimes
      • Supported Processor Architectures
    • Integrations

When it could be:

  • [Page title] .NET Core Compatibility Requirements
    • Supported .NET Core runtimes
    • Supported Processor Architectures
    • Integrations

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason I left the Compatibility section was to provide space for the following lines:

  • The .NET Tracer supports all .NET-based languages (for example, C#, F#, Visual Basic).

  • The .NET Tracer library for Datadog is open-source. For more information, see the tracer Github repository.

Otherwise they would not have a section header.

Comment on lines 18 to 20
<div class="alert alert-warning">
<strong>Notes:</strong><br><ul><li>Datadog automatic instrumentation relies on the .NET CLR Profiling API. This API allows only one subscriber (for example, APM). To ensure maximum visibility, run only one APM solution in your application environment.</li><li> If you are using both automatic and custom instrumentation, it is important to keep the package versions (for example, MSI and NuGet) in sync.</li></ul>
</div>
Copy link
Member

@lucaspimentel lucaspimentel Aug 10, 2021

Choose a reason for hiding this comment

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

[applies to both .NET Core and .NET Framework]

Do we really want this at the top of the page? It feels wrong to come to a compatibility page and the first thing you see is a yellow box with "don't do this!" warnings, instead of compatibility information.

(edit: typos)

Copy link
Contributor

Choose a reason for hiding this comment

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

+1, and since this is now brought to my attention, I'd suggest moving this from the compatibility page to the setup page. That would make the most sense to me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated on both .NET Core & Framework and moved this out of the compatibility page to the setup page under the Installation and getting started section.


## Integrations
### Supported Processor Architectures
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
### Supported Processor Architectures
### Supported processor architectures

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Replaced.


### Supported Processor Architectures
Copy link
Member

Choose a reason for hiding this comment

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

Sentence case, and copy over a line we have in .NET Core.

Suggested change
### Supported Processor Architectures
### Supported processor architectures
The .NET Tracer supports automatic instrumentation on the following architectures:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

replaced

Copy link
Contributor

@zacharycmontoya zacharycmontoya left a comment

Choose a reason for hiding this comment

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

I think Lucas has everything covered in his comments. Once those are addressed I'd say this is good to go.

Copy link
Member

@lucaspimentel lucaspimentel left a comment

Choose a reason for hiding this comment

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

thanks!

@cswatt cswatt merged commit ceec40f into master Aug 11, 2021
@cswatt cswatt deleted the jay.dave/dotnet_new_integrations branch August 11, 2021 19:38
2. In your application code, access the global tracer through the `Datadog.Trace.Tracer.Instance` property to create new spans.

For more details on custom instrumentation and custom tagging, see [.NET Custom Instrumentation documentation][4].
For more details on custom instrumentation and custom tagging, see [.NET Custom Instrumentation documentation][3].

## Install and configure the Agent for APM
Copy link
Contributor

Choose a reason for hiding this comment

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

@jaycdave88 as a note for later, I think we should follow the example of other tracers and include this section as a subsection of "Installation and getting started". This applies for both .NET Core and .NET Framework pages

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tracing Content changed in the tracing folder
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants