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

Consider upgrading DiagnosticSource dependency to 5.0 #33683

Closed
JoshLove-msft opened this issue Jan 26, 2023 · 9 comments
Closed

Consider upgrading DiagnosticSource dependency to 5.0 #33683

JoshLove-msft opened this issue Jan 26, 2023 · 9 comments
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library.
Milestone

Comments

@JoshLove-msft
Copy link
Member

JoshLove-msft commented Jan 26, 2023

We are currently using 4.6.0 for compatibility with V2 functions. At this point, almost all customers are on V3. In V3, the function host uses 5.0.0, so we should be able to safely upgrade from a functions perspective at least. There may also be a dependency consideration for PowerShell integration with our libraries, but would need to investigate the details there.

@JoshLove-msft
Copy link
Member Author

/cc @lmolkova

@JoshLove-msft JoshLove-msft added this to the Backlog milestone Jan 26, 2023
@JoshLove-msft
Copy link
Member Author

This would also help with testing the ActivitySource path. Otherwise test projects need to explicitly add a 5.0.0 dependency.

@jsquire jsquire added the Client This issue points to a problem in the data-plane of the library. label Jan 26, 2023
@lmolkova
Copy link
Member

It would also allow us to do this: Azure/azure-sdk#3319

@fowl2
Copy link

fowl2 commented Apr 28, 2023

I would hope there there would at least be deprecation warning before dropping support for Azure Functions v1! .NET Framework support for v4 only went GA very recently and it takes time to port.

@m-redding
Copy link
Member

Linking some old issues for additional context: #8739 #25824 #7166

@lmolkova
Copy link
Member

lmolkova commented Jun 15, 2023

thanks for digging up the old issues, @m-redding!

I think #8739 is really relevant , AFAIK PowerShell 6 is no longer supported and PowerShell 7 should have the right version of DiagnosticSource:

PS> [System.AppDomain]::CurrentDomain.GetAssemblies().where({$_.FullName -match "DiagnosticSource"}) | Select FullName

FullName

System.Diagnostics.DiagnosticSource, Version=7.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51

Perhaps, we can test if doing something simple in powershell with azure SDK works (if we update DS to 5.0.0)
As a precaution, I wonder if we could guard tracing code with a runtime check that would disable all instrumentation if DS version is lower than expected (or multiple DS assemblies are loaded as it happens on Functions).

@fowl2 I believe we won't stop supporting Functions v1 with this, but would stop supporting distributed tracing in newish Azure SDK on Functions v1 with such a change.

@m-redding
Copy link
Member

Ended up upgrading to 6.0.1 - PR #37418 has additional context/reasoning for this decision

@fowl2
Copy link

fowl2 commented Jul 14, 2023

@fowl2 I believe we won't stop supporting Functions v1 with this, but would stop supporting distributed tracing in newish Azure SDK on Functions v1 with such a change.

@lmolkova @m-redding could you confirm this won't break Azure Functions v1 (see #33683 (comment)) / if it is, it's clear in the release notes. Thanks!

@m-redding
Copy link
Member

Hi @fowl2! I had some discussions with the Functions team, and we don't believe this will break Functions V1 compatibility. The upgraded System.Diagnostics.DiagnosticSource package is compatible with .NET Framework 4.8. There's a chance you may get a warning from the compiler depending on how your project is configured, but the tracing code will continue to work as it did before.

@github-actions github-actions bot locked and limited conversation to collaborators Oct 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library.
Projects
None yet
Development

No branches or pull requests

5 participants