Enable .net6.0 build and add net6.0 target #2559

vishweshbankwar · 2021-11-02T20:41:18Z

Fixes #.
2064

Changes

Please provide a brief description of the changes here.

Add .NET6.0 build
Add .NET6.0 target to test projects and aspnetcore instrumentation library

todos

Replace RC package version to GA versions once available.
Remove explicit install for .NET SDKs once 6.0 is available as pre-installed.

For significant contributions please make sure you have completed the following items:

CHANGELOG.md updated for non-trivial changes
Design discussion issue #
Changes in public API reviewed

codecov · 2021-11-02T20:51:50Z

Codecov Report

Merging #2559 (80f27e7) into main (02f2594) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #2559   +/-   ##
=======================================
  Coverage   80.84%   80.84%           
=======================================
  Files         254      254           
  Lines        8552     8552           
=======================================
  Hits         6914     6914           
  Misses       1638     1638

Impacted Files	Coverage Δ
...ion.AspNetCore/AspNetCoreInstrumentationOptions.cs	`100.00% <ø> (ø)`
...emetry.Api/Internal/OpenTelemetryApiEventSource.cs	`79.41% <0.00%> (-2.95%)`	⬇️
...cSourceInstrumentation/DiagnosticSourceListener.cs	`80.00% <0.00%> (+5.00%)`	⬆️

.github/workflows/docfx.yml

reyang · 2021-11-04T00:18:10Z

(no need to make the change in this PR)

It seems most of the test projects are using the same target frameworks netcoreapp3.1;net5.0;net6.0, might be good to extract the logic to a common place like

opentelemetry-dotnet/docs/Directory.Build.props

Line 8 in 02f2594

<TargetFrameworks>netcoreapp3.1;net5.0</TargetFrameworks>

.

eddynaka · 2021-11-04T00:11:59Z

.github/workflows/code-coverage.yml

@@ -26,6 +26,19 @@ jobs:
      with:
        fetch-depth: 0 # fetching all

+    - uses: actions/setup-dotnet@v1


we don't need to install 3.1.x/5.0.x because they are already installed in windows-latest. I always check this:

https://github.com/actions/virtual-environments

https://github.com/actions/virtual-environments/blob/main/images/win/Windows2019-Readme.md

Remember that windows-latest can change in the future to windows-2022, so, always have that in mind.

eddynaka · 2021-11-04T00:14:21Z

.github/workflows/integration.yml

    steps:
      - uses: actions/checkout@v2
+
+      - uses: actions/setup-dotnet@v1


same comment as above, no need for 3.1.x or 5.0.x

eddynaka · 2021-11-04T00:14:36Z

.github/workflows/integration.yml

    steps:
      - uses: actions/checkout@v2
+
+      - uses: actions/setup-dotnet@v1


here as well :)

eddynaka · 2021-11-04T00:14:48Z

.github/workflows/integration.yml

    steps:
      - uses: actions/checkout@v2
+
+      - uses: actions/setup-dotnet@v1


here as well

eddynaka · 2021-11-04T00:15:33Z

.github/workflows/integration.yml

    steps:
      - uses: actions/checkout@v2
+
+      - uses: actions/setup-dotnet@v1


same here.
you can check this:
https://github.com/actions/virtual-environments/blob/main/images/linux/Ubuntu2004-README.md

eddynaka · 2021-11-04T00:15:44Z

.github/workflows/linux-ci.yml


    steps:
    - uses: actions/checkout@v2

+    - uses: actions/setup-dotnet@v1


same here:
https://github.com/actions/virtual-environments/blob/main/images/linux/Ubuntu2004-README.md

eddynaka · 2021-11-04T00:15:59Z

.github/workflows/windows-ci.yml


    steps:
    - uses: actions/checkout@v2

+    - uses: actions/setup-dotnet@v1


eddynaka · 2021-11-04T00:18:26Z

test/TestApp.AspNetCore.6.0/Controllers/ChildActivityController.cs

+using Microsoft.AspNetCore.Mvc;
+using OpenTelemetry;
+
+namespace TestApp.AspNetCore._6._0.Controllers


one day, we have to figure out how to prevent this code over and over.
we already have the "same" code for 3.1, 5.0, 6.0

*.tt files? 🤣

https://docs.microsoft.com/visualstudio/modeling/code-generation-and-t4-text-templates

vishweshbankwar · 2021-11-04T00:25:03Z

(no need to make the change in this PR)

It seems most of the test projects are using the same target frameworks netcoreapp3.1;net5.0;net6.0, might be good to extract the logic to a common place like

opentelemetry-dotnet/docs/Directory.Build.props

Line 8 in 02f2594

<TargetFrameworks>netcoreapp3.1;net5.0</TargetFrameworks>

.

Yes - I was thinking the same. will look in to creating a separate PR for this.

CodeBlanch · 2021-11-04T17:32:05Z

src/OpenTelemetry.Instrumentation.AspNetCore/OpenTelemetry.Instrumentation.AspNetCore.csproj

@@ -1,6 +1,6 @@
 <Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
-    <TargetFrameworks>netstandard2.0;netstandard2.1;netcoreapp3.1;net5.0</TargetFrameworks>
+    <TargetFrameworks>netstandard2.0;netstandard2.1;netcoreapp3.1;net5.0;net6.0</TargetFrameworks>


Just curious, why was this needed?

As we are using FrameworkReference, this is needed.
IIRC, if we do not add this, net5.0 targeted instrumentation library will be used for net6.0 app (should still work as we don't have any NET6 framework specific code).

I don't think it is needed until we want to use some piece of the .NET 6 API.

If the project is like this...

<TargetFrameworks>netstandard2.0;netstandard2.1;netcoreapp3.1;net5.0</TargetFrameworks> <ItemGroup Condition="'$(TargetFramework)' == 'net5.0'"> <FrameworkReference Include="Microsoft.AspNetCore.App" /> </ItemGroup>

Users on .NET 6 will still be able to use it.

Agreed - then do we remove 5.0 as well?. I have not tested but I think having only 3.1 will also work for 3.1+ apps then?.
But if we ever use version specific code, we will have to do add it again.
@cijothomas - thoughts?

We don't use any .NET 6 specific feature today, but Asp.Net core 6.0 has introduced ActivitySource usage. If we want to leverage them, we either need .net6.0 target. or we use reflection to find if user is running Asp.Net Core 6.0 or newer.
(@vishweshbankwar please create a separate issue to discuss this further)

bruno-garcia · 2021-11-04T18:08:50Z

src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationOptions.cs

@@ -52,7 +52,7 @@ public class AspNetCoreInstrumentationOptions
        /// </remarks>
        public bool RecordException { get; set; }

-#if NETSTANDARD2_1 || NETCOREAPP3_1 || NET5_0
+#if NETSTANDARD2_1 || NETCOREAPP3_1 || NET5_0_OR_GREATER


Can be only:

#if NETSTANDARD2_1 || NETCOREAPP3_1_OR_GREATER

The reason why we probably used #if NETSTANDARD2_1 || NETCOREAPP3_1 || NET5_0 in the first place
https://docs.microsoft.com/en-us/dotnet/core/compatibility/sdk/5.0/netcoreapp3_1-preprocessor-symbol-not-defined

That affects .net 5 SDK, right? So now on NET6 it should work if I undersand?

Yes, it looks like it from the .NET 5 and later bullet here

Vishwesh Bankwar added 3 commits November 2, 2021 13:40

add .net6.0 to ci

da44deb

add docker compose for 6.0

7cb7c61

add dotnet 6

4bb1615

Vishwesh Bankwar and others added 11 commits November 2, 2021 13:52

add set up step

95fe250

add net6.0 to sql client test

ca7481b

rc sdk version

539e1e8

revert dotnet set up

aee44ae

add dotnet install

04f9d06

integration

cdf0b75

rc sdk version

241302c

update docker version

de1282b

Merge branch 'main' into vibankwa/Add-DotNet6-CI

725e7df

update mcr

a7d0321

fix otlp docker

186d7f2

vishweshbankwar closed this Nov 2, 2021

vishweshbankwar reopened this Nov 2, 2021

Vishwesh Bankwar added 12 commits November 2, 2021 16:42

add 6.0 to api compat

edfd114

update sln

59d1e18

consistent preprocessor dir

8968dc2

add net6.0 to codcov

399c931

resolve package conflicts

19ed125

doc fx fix

ed12ef6

revert linux and windows

89c0b24

minor fix

a80c37a

revert

4bd2f42

test with only net6 install

107e0e2

revert

9c0a502

add 3.1 and 5 to codcov

cf048c5

vishweshbankwar changed the title ~~Enable .net6.0 build~~ Enable .net6.0 build and add net6.0 target Nov 3, 2021

Vishwesh Bankwar added 2 commits November 3, 2021 15:12

test codcov without install

9e0710b

revert codecov

eb600f3

vishweshbankwar marked this pull request as ready for review November 3, 2021 22:33

vishweshbankwar requested a review from a team November 3, 2021 22:33

cijothomas reviewed Nov 3, 2021

View reviewed changes

.github/workflows/docfx.yml Show resolved Hide resolved

Vishwesh Bankwar added 3 commits November 3, 2021 16:29

revert windows

559bca4

add back 3.1 and 5.0

7cdf850

add comment for specifying docfx version

68b2e46

reyang approved these changes Nov 4, 2021

View reviewed changes

eddynaka reviewed Nov 4, 2021

View reviewed changes

trying fix to avoid 3.1 and 5.0 install

86f5658

eddynaka approved these changes Nov 4, 2021

View reviewed changes

Vishwesh Bankwar added 6 commits November 3, 2021 17:53

fix to avoid installing 3.1 and 5.0

1fb2495

fail-fast false

828f8a3

minor

e4b12ec

fix linux-ci formatting

1e51043

try setting env at test step

2381758

fix apicompat, codecov and linux

80f27e7

cijothomas approved these changes Nov 4, 2021

View reviewed changes

cijothomas merged commit 4f930d5 into open-telemetry:main Nov 4, 2021

CodeBlanch reviewed Nov 4, 2021

View reviewed changes

bruno-garcia reviewed Nov 4, 2021

View reviewed changes

vishweshbankwar mentioned this pull request Mar 16, 2022

REQUEST: New membership for vishweshbankwar open-telemetry/community#1005

Closed

6 tasks

TimothyMothra mentioned this pull request Nov 23, 2022

remove pinned version of docfx #3935

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable .net6.0 build and add net6.0 target #2559

Enable .net6.0 build and add net6.0 target #2559

vishweshbankwar commented Nov 2, 2021 •

edited

Loading

codecov bot commented Nov 2, 2021 •

edited

Loading

reyang commented Nov 4, 2021

eddynaka Nov 4, 2021

eddynaka Nov 4, 2021

eddynaka Nov 4, 2021

eddynaka Nov 4, 2021

eddynaka Nov 4, 2021

eddynaka Nov 4, 2021

eddynaka Nov 4, 2021

eddynaka Nov 4, 2021

reyang Nov 4, 2021

vishweshbankwar commented Nov 4, 2021

CodeBlanch Nov 4, 2021

vishweshbankwar Nov 4, 2021 •

edited

Loading

CodeBlanch Nov 4, 2021

vishweshbankwar Nov 4, 2021

cijothomas Nov 4, 2021

bruno-garcia Nov 4, 2021

mic-max Nov 4, 2021

bruno-garcia Nov 4, 2021

mic-max Nov 4, 2021

Enable .net6.0 build and add net6.0 target #2559

Enable .net6.0 build and add net6.0 target #2559

Conversation

vishweshbankwar commented Nov 2, 2021 • edited Loading

Changes

codecov bot commented Nov 2, 2021 • edited Loading

Codecov Report

reyang commented Nov 4, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vishweshbankwar commented Nov 4, 2021

Choose a reason for hiding this comment

vishweshbankwar Nov 4, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vishweshbankwar commented Nov 2, 2021 •

edited

Loading

codecov bot commented Nov 2, 2021 •

edited

Loading

vishweshbankwar Nov 4, 2021 •

edited

Loading