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

[Not for merging] netcoreapp3.1 for Prometheus middleware project #3559

Closed

Conversation

alanwest
Copy link
Member

@alanwest alanwest commented Aug 5, 2022

No need for merging or making a decision at the moment. Opening this PR just to illustrate things, but I plan to close it and just open an issue.

This needs to be an additional discussion point as we consider how to handle #3448.

The Prometheus ASP.NET Core middleware project previously targeted netcoreapp3.1. Upon merging our net7.0 branch, it now just targets net6.0.

Do we only want to support net6.0+ with our Prometheus exporter?

Another option may be to "support" it in the way this PR proposes by suppressing the build warnings for the purposes of our build, though I believe this would still result in end users getting the warnings.

Maybe we'd take the stance that we don't really support the netcoreapp3.1 version even though we offer a version that targets it?

@alanwest alanwest requested a review from a team August 5, 2022 23:03
@codecov
Copy link

codecov bot commented Aug 5, 2022

Codecov Report

Merging #3559 (2032c16) into main (72f4e07) will increase coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3559      +/-   ##
==========================================
+ Coverage   87.23%   87.27%   +0.04%     
==========================================
  Files         275      275              
  Lines        9959     9959              
==========================================
+ Hits         8688     8692       +4     
+ Misses       1271     1267       -4     
Impacted Files Coverage Δ
src/OpenTelemetry.Api/Internal/Guard.cs 0.00% <ø> (ø)
...Listener/Internal/PrometheusExporterEventSource.cs 16.66% <0.00%> (-11.12%) ⬇️
....Prometheus.HttpListener/PrometheusHttpListener.cs 78.66% <0.00%> (-4.00%) ⬇️
...tpListener/Internal/PrometheusCollectionManager.cs 78.04% <0.00%> (-2.44%) ⬇️
...metryProtocol/Implementation/ActivityExtensions.cs 94.50% <0.00%> (-1.10%) ⬇️
...nTelemetry/Internal/OpenTelemetrySdkEventSource.cs 77.50% <0.00%> (+0.83%) ⬆️
src/OpenTelemetry/BatchExportProcessor.cs 84.11% <0.00%> (+1.86%) ⬆️
...tation/OpenTelemetryProtocolExporterEventSource.cs 85.00% <0.00%> (+10.00%) ⬆️
src/OpenTelemetry/Logs/Pool/LogRecordSharedPool.cs 100.00% <0.00%> (+21.05%) ⬆️

@alanwest
Copy link
Member Author

alanwest commented Aug 5, 2022

Closing to continue discussion on #3560

@alanwest alanwest closed this Aug 5, 2022
@alanwest alanwest deleted the alanwest/netcoreapp3.1-prometheus branch August 5, 2022 23:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant