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

Azure.Monitor.OpenTelemetry.Exporter does not support net461 #33970

Closed
Tracked by #33969
heaths opened this issue Feb 8, 2023 · 8 comments
Closed
Tracked by #33969

Azure.Monitor.OpenTelemetry.Exporter does not support net461 #33970

heaths opened this issue Feb 8, 2023 · 8 comments
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. EngSys This issue is impacting the engineering system. Monitor Monitor, Monitor Ingestion, Monitor Query

Comments

@heaths
Copy link
Member

heaths commented Feb 8, 2023

When building everything in the net - core - ci pipeline, we found that Azure.Monitor.OpenTelemetry.Exporter tests, demos, and benchmarks are erring,

C:\Users\heaths.nuget\packages\system.diagnostics.diagnosticsource\7.0.0\buildTransitive\net461\System.Diagnostics.DiagnosticSource.targets(4,5): System.Diagnostics.DiagnosticSource 7.0.0 doesn't support net461 and has not been tested with it. Consider upgrading your TargetFramework to net462 or later. You may also set true in the project file to ignore this warning and attempt to run in this unsupported configuration at your own risk. [C:\src\azure-sdk-for-net\sdk\monitor\Azure.Monitor.OpenTelemetry.Exporter\tests\Azure.Monitor.OpenTelemetry.Exporter.Benchmarks\Azure.Monitor.OpenTelemetry.Exporter.Benchmarks.csproj]

Note that guidelines state:

DO build all libraries for .NET Standard 2.0.

Exceptions can be made - and in this case may have to since System.Diagnostics.DiagnosticSource does not support net461 - but that needs to be excluded from the build then.

@heaths heaths added Monitor Monitor, Monitor Ingestion, Monitor Query Client This issue points to a problem in the data-plane of the library. EngSys This issue is impacting the engineering system. labels Feb 8, 2023
@heaths heaths mentioned this issue Feb 8, 2023
19 tasks
@heaths
Copy link
Member Author

heaths commented Feb 8, 2023

Turns out that dotnet test eng/service.proj -warnaserror /t:rebuild /p:DebugType=none /p:SDKType=all /p:ServiceDirectory=monitor /p:IncludePerf=false /p:IncludeStress=false /p:PublicSign=false /p:Configuration=Release /p:EnableSourceLink=false /p:BuildSnippets=true works if we add the following to the Directory.Build.props file:

<SuppressTfmSupportBuildWarnings>true</SuppressTfmSupportBuildWarnings>

I leave it up to the owning team to decide if they want to seek an exception and change this to target net462 instead, or leave this "workaround" in place as-is and resolve this bug.

@TimothyMothra TimothyMothra self-assigned this Feb 8, 2023
@TimothyMothra
Copy link
Contributor

Related conversation: open-telemetry/opentelemetry-dotnet#3448

@TimothyMothra
Copy link
Contributor

We may want to explore this from .NET:

For libraries that support .NETStandard, the .NETStandard Compatibility packaging infrastructure makes sure that out-of-support target frameworks like netcoreapp3.1 or net461 are unsupported by the produced package. That enables library authors to support .NETStandard but explicitly not support unsupported .NETStandard compatible target frameworks. The infrastructure generates a targets file that throws a user readable Error when msbuild invokes a project with an unsupported target framework.

https://github.com/dotnet/runtime/blob/main/docs/coding-guidelines/libraries-packaging.md#netstandard-compatibility-error-infrastructure

@KrzysztofCwalina
Copy link
Member

I cannot parse "unsupported .NETStandard compatible target frameworks." /s

Since the APIs exists, maybe we can just not call them at runtime if we detect that they would have throw PlatfromNotSupportedException.

@heaths
Copy link
Member Author

heaths commented Feb 10, 2023

Since the APIs exists, maybe we can just not call them at runtime if we detect that they would have throw PlatfromNotSupportedException.

This is what I do in Key Vault where the API is defined but may not be supported by net461. It requires runtime detection, however. In my case I'm using reflection (cryptography was the biggest area where netstandard2.0 claimed APIs but net461 (and, in some cases, net462) didn't actually define them), but there may be other plausible solutions depending on the scenario.

@KrzysztofCwalina
Copy link
Member

Possibly there are other solutions, but I am not a fan of marking package as NS2 compliant and then saying that one of the "compatible frameworks" is not supported and so the build fails. If we could make it a warning, i.e. "On 461 OpenTelemetry will not work", then it would be ideal.

@TimothyMothra
Copy link
Contributor

I cannot parse "unsupported .NETStandard compatible target frameworks." /s

I would defer to the dotnet team on this matter. net461 is compatible with netstandard2.0. net461 is also unsupported. Both of these statements are true.

If we could make it a warning, i.e. "On 461 OpenTelemetry will not work", then it would be ideal.

I think this is reasonable and seems to be in line with what dotnet is doing. System.Diagnostics.DiagnosticSource v7.0.0 is throwing the build warning quoted above. This became an issue when using the -warnaserror flag.

@TimothyMothra
Copy link
Contributor

I think this issue can be closed.
net461 has been removed from the test matrix: #35978

@github-actions github-actions bot locked and limited conversation to collaborators Aug 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. EngSys This issue is impacting the engineering system. Monitor Monitor, Monitor Ingestion, Monitor Query
Projects
None yet
Development

No branches or pull requests

3 participants