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

[repo] Replace .NET6 target with .NET9 #5832

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 21 additions & 17 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ csharp_indent_labels = flush_left

# Modifier preferences
csharp_preferred_modifier_order = public,private,protected,internal,file,static,extern,new,virtual,abstract,sealed,override,readonly,unsafe,required,volatile,async:suggestion
dotnet_style_require_accessibility_modifiers = for_non_interface_members:silent
dotnet_style_require_accessibility_modifiers = for_non_interface_members:suggestion

# this. preferences
dotnet_style_qualification_for_field = true:suggestion
Expand All @@ -53,8 +53,8 @@ dotnet_style_qualification_for_event = true:suggestion
csharp_style_var_for_built_in_types = true:silent
csharp_style_var_when_type_is_apparent = true:silent
csharp_style_var_elsewhere = true:silent
dotnet_style_predefined_type_for_locals_parameters_members = true:silent
dotnet_style_predefined_type_for_member_access = true:silent
dotnet_style_predefined_type_for_locals_parameters_members = true:suggestion
dotnet_style_predefined_type_for_member_access = true:suggestion

# name all constant fields using PascalCase
dotnet_naming_rule.constant_fields_should_be_pascal_case.severity = suggestion
Expand All @@ -75,28 +75,31 @@ dotnet_style_readonly_field = true:suggestion
csharp_style_implicit_object_creation_when_type_is_apparent = true:suggestion
dotnet_style_prefer_simplified_interpolation = true:suggestion
dotnet_style_object_initializer = true:suggestion
csharp_style_prefer_primary_constructors = false:none

# Expression-level preferences
dotnet_style_object_initializer = true:suggestion
dotnet_style_collection_initializer = true:suggestion
dotnet_style_explicit_tuple_names = true:suggestion
dotnet_style_coalesce_expression = true:suggestion
dotnet_style_null_propagation = true:suggestion
dotnet_style_prefer_is_null_check_over_reference_equality_method = true:silent
dotnet_style_prefer_is_null_check_over_reference_equality_method = true:suggestion
dotnet_style_prefer_inferred_tuple_names = true:suggestion
dotnet_style_prefer_inferred_anonymous_type_member_names = true:suggestion
dotnet_style_prefer_auto_properties = true:silent
dotnet_style_prefer_conditional_expression_over_assignment = true:silent
dotnet_style_prefer_conditional_expression_over_return = true:silent
dotnet_style_prefer_auto_properties = true:suggestion
dotnet_style_prefer_conditional_expression_over_assignment = true:suggestion
dotnet_style_prefer_conditional_expression_over_return = true:suggestion
csharp_prefer_simple_default_expression = true:suggestion
csharp_style_unused_value_expression_statement_preference = discard_variable:none

# Expression-bodied members
csharp_style_expression_bodied_methods = false:silent
csharp_style_expression_bodied_constructors = false:silent
csharp_style_expression_bodied_operators = false:silent
csharp_style_expression_bodied_properties = true:silent
csharp_style_expression_bodied_indexers = true:silent
csharp_style_expression_bodied_accessors = true:silent
csharp_style_expression_bodied_methods = true:suggestion
dotnet_diagnostic.IDE0022.severity = suggestion # dotnet format doesn't respect the suggestion in the line above
csharp_style_expression_bodied_constructors = false:warning
csharp_style_expression_bodied_operators = true:suggestion
csharp_style_expression_bodied_properties = true:suggestion
csharp_style_expression_bodied_indexers = true:suggestion
csharp_style_expression_bodied_accessors = true:suggestion

# Pattern matching
csharp_style_pattern_matching_over_is_with_cast_check = true:suggestion
Expand All @@ -113,6 +116,7 @@ csharp_style_prefer_range_operator = false:none
csharp_style_pattern_local_over_anonymous_function = true:suggestion
csharp_style_deconstructed_variable_declaration = true:suggestion
csharp_style_namespace_declarations = file_scoped:warning
dotnet_style_namespace_match_folder = false:none

# Space preferences
csharp_space_after_cast = false
Expand All @@ -128,10 +132,10 @@ csharp_space_between_method_declaration_parameter_list_parentheses = false
csharp_space_between_parentheses = false

# Parentheses preferences
dotnet_style_parentheses_in_arithmetic_binary_operators = always_for_clarity:silent
dotnet_style_parentheses_in_relational_binary_operators = always_for_clarity:silent
dotnet_style_parentheses_in_other_binary_operators = always_for_clarity:silent
dotnet_style_parentheses_in_other_operators = never_if_unnecessary:silent
dotnet_style_parentheses_in_arithmetic_binary_operators = always_for_clarity:suggestion
dotnet_style_parentheses_in_relational_binary_operators = always_for_clarity:suggestion
dotnet_style_parentheses_in_other_binary_operators = always_for_clarity:suggestion
dotnet_style_parentheses_in_other_operators = never_if_unnecessary:suggestion

# Code analyzers
# CA1031: Do not catch general exception types
Expand Down
2 changes: 1 addition & 1 deletion .github/ISSUE_TEMPLATE/bug_report.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ body:
- type: input
attributes:
label: Runtime Version
description: What .NET runtime version did you use? (e.g. `net462`, `net48`, `netcoreapp3.1`, `net6.0` etc. You can find this information from the `*.csproj` file)
description: What .NET runtime version did you use? (e.g. `net462`, `net48`, `net8.0`, etc. You can find this information from the `*.csproj` file)
validations:
required: true

Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/Component.BuildTest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ on:
required: false
type: string
tfm-list:
default: '[ "net462", "net6.0", "net8.0" ]'
default: '[ "net462", "net8.0", "net9.0" ]'
required: false
type: string

Expand All @@ -42,7 +42,7 @@ jobs:
- os: otel-linux-arm64
version: net462
- os: otel-linux-arm64
version: net6.0
version: net8.0

runs-on: ${{ matrix.os }}
steps:
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ jobs:
strategy:
fail-fast: false
matrix:
version: [ net6.0, net8.0 ]
version: [ net8.0, net9.0 ]
steps:
- uses: actions/checkout@v4
- name: Run OTLP Exporter docker compose
Expand All @@ -129,7 +129,7 @@ jobs:
strategy:
fail-fast: false
matrix:
version: [ net6.0, net8.0 ]
version: [ net8.0, net9.0 ]
steps:
- uses: actions/checkout@v4
- name: Run W3C Trace Context docker compose
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/docfx.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ jobs:
- name: check out code
uses: actions/checkout@v4

- name: Setup dotnet
uses: actions/setup-dotnet@v4

- name: install docfx
run: dotnet tool install -g docfx

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/verifyaotcompat.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ jobs:
fail-fast: false # ensures the entire test matrix is run, even if one permutation fails
matrix:
os: [ ubuntu-latest, windows-latest ]
version: [ net8.0 ]
version: [ net8.0, net9.0 ]

runs-on: ${{ matrix.os }}
steps:
Expand Down
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ You can contribute to this project from a Windows, macOS or Linux machine.
On all platforms, the minimum requirements are:

* Git client and command line tools.
* .NET 8.0
* .NET 9.0

### Linux or MacOS

Expand Down
6 changes: 3 additions & 3 deletions Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,10 @@
<PackageVersion Include="xunit.runner.visualstudio" Version="[2.8.2,3.0)" />
</ItemGroup>

<ItemGroup Condition="'$(TargetFramework)' == 'net6.0'">
<PackageVersion Include="Microsoft.AspNetCore.TestHost" Version="6.0.33" />
</ItemGroup>
<ItemGroup Condition="'$(TargetFramework)' == 'net8.0'">
<PackageVersion Include="Microsoft.AspNetCore.TestHost" Version="8.0.8" />
</ItemGroup>
<ItemGroup Condition="'$(TargetFramework)' == 'net9.0'">
<PackageVersion Include="Microsoft.AspNetCore.TestHost" Version="9.0.0-rc.1.24452.1" />
</ItemGroup>
</Project>
2 changes: 1 addition & 1 deletion OpenTelemetry.sln
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "build", "build", "{7CB2F02E
build\debug.snk = build\debug.snk
Directory.Packages.props = Directory.Packages.props
build\docfx.cmd = build\docfx.cmd
build\docker-compose.net6.0.yml = build\docker-compose.net6.0.yml
build\docker-compose.net8.0.yml = build\docker-compose.net8.0.yml
build\docker-compose.net9.0.yml = build\docker-compose.net9.0.yml
build\GlobalAttrExclusions.txt = build\GlobalAttrExclusions.txt
build\opentelemetry-icon-color.png = build\opentelemetry-icon-color.png
build\OpenTelemetry.prod.loose.ruleset = build\OpenTelemetry.prod.loose.ruleset
Expand Down
2 changes: 1 addition & 1 deletion build/Common.nonprod.props
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
</PropertyGroup>

<PropertyGroup>
<DefaultTargetFrameworkForExampleApps>net8.0</DefaultTargetFrameworkForExampleApps>
<DefaultTargetFrameworkForExampleApps>net9.0</DefaultTargetFrameworkForExampleApps>
rajkumar-rangaraj marked this conversation as resolved.
Show resolved Hide resolved
</PropertyGroup>

<PropertyGroup Condition="$(MSBuildProjectName.EndsWith('.Tests'))">
Expand Down
7 changes: 7 additions & 0 deletions build/Common.prod.props
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@
<ContinuousIntegrationBuild Condition="'$(Deterministic)'=='true'">true</ContinuousIntegrationBuild>
</PropertyGroup>

<ItemGroup>
<!-- Note: Disable net6.0 target for package validation because it has been
removed. It should be possible to remove this once a stable version has been
released to NuGet without net6.0. -->
<PackageValidationBaselineFrameworkToIgnore Include="net6.0" />
</ItemGroup>

<ItemGroup>
<PackageReference Include="MinVer" PrivateAssets="All" Condition="'$(IntegrationBuild)' != 'true'" />
<PackageReference Include="Microsoft.SourceLink.GitHub" PrivateAssets="All" Condition="'$(IntegrationBuild)' != 'true'" />
Expand Down
14 changes: 7 additions & 7 deletions build/Common.props
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,18 @@
<NetFrameworkSupportedVersions>net481;net48;net472;net471;net47;net462</NetFrameworkSupportedVersions>

<!-- production TFMs -->
<TargetFrameworksForLibraries>net8.0;net6.0;netstandard2.0;$(NetFrameworkMinimumSupportedVersion)</TargetFrameworksForLibraries>
<TargetFrameworksForLibrariesExtended>net8.0;net6.0;netstandard2.1;netstandard2.0;$(NetFrameworkMinimumSupportedVersion)</TargetFrameworksForLibrariesExtended>
<TargetFrameworksForPrometheusAspNetCore>net8.0;net6.0</TargetFrameworksForPrometheusAspNetCore>
<TargetFrameworksForLibraries>net9.0;net8.0;netstandard2.0;$(NetFrameworkMinimumSupportedVersion)</TargetFrameworksForLibraries>
<TargetFrameworksForLibrariesExtended>net9.0;net8.0;netstandard2.1;netstandard2.0;$(NetFrameworkMinimumSupportedVersion)</TargetFrameworksForLibrariesExtended>
<TargetFrameworksForPrometheusAspNetCore>net9.0;net8.0</TargetFrameworksForPrometheusAspNetCore>
Comment on lines +31 to +33
Copy link
Contributor

Choose a reason for hiding this comment

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

One more thing, should we really add there net9? .NET8 will be supported longer than .NET8.
Previously there were no packages targeted .NET7. Ref: #5795

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@CodeBlanch What are your thoughts on it?

Copy link
Member

Choose a reason for hiding this comment

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

I don't see any particular advantage with a net9 target...

Copy link
Member

@CodeBlanch CodeBlanch Sep 19, 2024

Choose a reason for hiding this comment

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

Reminds me of this: #4918

Question being, why did we add net8.0 to everything? 🤣 At the time I tried to block that because it exploded the amount of public API files we had and maintaining those is a pain. But I went and changed how the public API stuff works so we don't need public API files for a TFM unless there are customizations for that TFM.

Where we are now I don't have an issue with adding net9.0. We did it for net8.0. Which I guess is to say I'm fine with each year adding the latest version as a target when we upgrade. Don't have a strong reason for that other than when users look at NuGet and they see explicit targets I guess it is strong indication we were intentional 🤷

/cc @alanwest

Copy link
Contributor

Choose a reason for hiding this comment

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

Was net8.0 more valid to add because it's a LTS release? net7.0 end of support is before net6.0, so it'll be removed before net6.0 anyways. net8.0, however, will outlive net6.0. Not adding net8.0 will unavoidably leave a gap somewhere in the timeline.

image

Copy link
Member

Choose a reason for hiding this comment

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

I don't see any particular advantage with a net9 target...

Me either, and as @CodeBlanch points out this is something we have pushed against before, and honestly I never understood the reasons for adding a bunch of targets.

It stems back to this comment #4591 (comment). But again, I never understood the benefits to the OpenTelemetry .NET project.

All that said, I'm fine just adding net9.0 everywhere and continuing to make that the pattern going forward. If we want to circle back to this decision then I'd prefer it be in the context of a conversation about removing all unnecessary targets and going back to the state we had before.


<!-- non-production TFMs -->
<TargetFrameworksForAspNetCoreTests>net8.0;net6.0</TargetFrameworksForAspNetCoreTests>
<TargetFrameworksForAotCompatibilityTests>net8.0</TargetFrameworksForAotCompatibilityTests>
<TargetFrameworksForDocs>net8.0;net6.0</TargetFrameworksForDocs>
<TargetFrameworksForAspNetCoreTests>net9.0;net8.0</TargetFrameworksForAspNetCoreTests>
<TargetFrameworksForAotCompatibilityTests>net9.0;net8.0</TargetFrameworksForAotCompatibilityTests>
<TargetFrameworksForDocs>net9.0;net8.0</TargetFrameworksForDocs>
<TargetFrameworksForDocs Condition="$(OS) == 'Windows_NT' And '$(UsingMicrosoftNETSdkWeb)' != 'True'">
$(TargetFrameworksForDocs);$(NetFrameworkSupportedVersions)
</TargetFrameworksForDocs>
<TargetFrameworksForTests>net8.0;net6.0</TargetFrameworksForTests>
<TargetFrameworksForTests>net9.0;net8.0</TargetFrameworksForTests>
<TargetFrameworksForTests Condition="$(OS) == 'Windows_NT'">
$(TargetFrameworksForTests);$(NetFrameworkMinimumSupportedVersion)
</TargetFrameworksForTests>
Expand Down
9 changes: 0 additions & 9 deletions build/docker-compose.net6.0.yml

This file was deleted.

2 changes: 1 addition & 1 deletion build/docker-compose.net8.0.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@ services:
args:
PUBLISH_FRAMEWORK: net8.0
TEST_SDK_VERSION: "8.0"
BUILD_SDK_VERSION: "8.0"
BUILD_SDK_VERSION: "9.0"
9 changes: 9 additions & 0 deletions build/docker-compose.net9.0.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
version: '3.7'

services:
tests:
build:
args:
PUBLISH_FRAMEWORK: net9.0
TEST_SDK_VERSION: "9.0"
BUILD_SDK_VERSION: "9.0"
2 changes: 1 addition & 1 deletion global.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"sdk": {
"rollForward": "latestFeature",
"version": "8.0.100"
"version": "9.0.100-rc.1.24452.12"
}
}
4 changes: 3 additions & 1 deletion src/OpenTelemetry/Internal/SelfDiagnosticsConfigParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,9 @@ public bool TryGetConfiguration(
this.configBuffer = buffer;
}

file.Read(buffer, 0, buffer.Length);
// TODO: Fix CA2022 - Avoid inexact read with 'System.IO.FileStream.Read(byte[], int, int)'
rajkumar-rangaraj marked this conversation as resolved.
Show resolved Hide resolved
// Added _ = as a workaround to suppress the warning
_ = file.Read(buffer, 0, buffer.Length);
string configJson = Encoding.UTF8.GetString(buffer);

if (!TryParseLogDirectory(configJson, out logDirectory))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
# This should be run from the root of the repo:
# docker build --file test/OpenTelemetry.Instrumentation.W3cTraceContext.Tests/Dockerfile .

ARG BUILD_SDK_VERSION=8.0
ARG TEST_SDK_VERSION=8.0
ARG BUILD_SDK_VERSION=9.0
rajkumar-rangaraj marked this conversation as resolved.
Show resolved Hide resolved
ARG TEST_SDK_VERSION=9.0

FROM ubuntu AS w3c
#Install git
Expand All @@ -13,7 +13,7 @@ RUN git clone --branch level-1 https://github.com/w3c/trace-context.git

FROM mcr.microsoft.com/dotnet/sdk:${BUILD_SDK_VERSION} AS build
ARG PUBLISH_CONFIGURATION=Release
ARG PUBLISH_FRAMEWORK=net8.0
ARG PUBLISH_FRAMEWORK=net9.0
WORKDIR /repo
COPY . ./
WORKDIR "/repo/test/OpenTelemetry.Instrumentation.W3cTraceContext.Tests"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,9 @@ public void SelfDiagnosticsEventListener_EmitEvent_OmitAsConfigured()

using FileStream file = File.Open(LOGFILEPATH, FileMode.Open, FileAccess.Read, FileShare.ReadWrite | FileShare.Delete);
var buffer = new byte[256];
file.Read(buffer, 0, buffer.Length);

// Suppress CA2022 error: Avoid inexact read with 'System.IO.FileStream.Read(byte[], int, int)'
_ = file.Read(buffer, 0, buffer.Length);
Assert.Equal('\0', (char)buffer[0]);
}

Expand Down Expand Up @@ -256,7 +258,9 @@ private static void AssertFileOutput(string filePath, string eventMessage)
{
using FileStream file = File.Open(filePath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite | FileShare.Delete);
var buffer = new byte[256];
file.Read(buffer, 0, buffer.Length);

// Suppress CA2022 error: Avoid inexact read with 'System.IO.FileStream.Read(byte[], int, int)'
_ = file.Read(buffer, 0, buffer.Length);
string logLine = Encoding.UTF8.GetString(buffer);
string logMessage = ParseLogMessage(logLine);
Assert.StartsWith(eventMessage, logMessage);
Expand Down
Loading