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

More OpenTelemetry changes #18246

Merged
merged 16 commits into from
Feb 10, 2025
Merged

More OpenTelemetry changes #18246

merged 16 commits into from
Feb 10, 2025

Conversation

majocha
Copy link
Contributor

@majocha majocha commented Jan 18, 2025

  • Add activity wrapping a test run to easily distinguish traces generated from testhost. Group by assembly + net framework.
  • log point-in-time events as events with data instead of activities to declutter traces. (for example StackGuard.Guard)
  • Enable trace provider in vsix when build in debug configuration: For example build VisualFSharpDebug in debug mode and start an instance with "Start WIthout Debugging" will produce traces viewable in Jeager.

Captured test run, some tests in parallel, a theory runs sequentially, Stackguard events with tags:
image

Traces from running VS instance:
image

Copy link
Contributor

github-actions bot commented Jan 18, 2025

⚠️ Release notes required, but author opted out

Warning

Author opted out of release notes, check is disabled for this pull request.
cc @dotnet/fsharp-team-msft

@majocha
Copy link
Contributor Author

majocha commented Jan 29, 2025

Slightly less allocations.

main:

Method Mean Error StdDev Gen0 Gen1 Gen2 Allocated
ParseAndCheckFileInProject 1.654 s 0.0225 s 0.0199 s 10000.0000 5000.0000 1000.0000 1.96 GB

this PR:

Method Mean Error StdDev Gen0 Gen1 Gen2 Allocated
ParseAndCheckFileInProject 1.625 s 0.0302 s 0.0297 s 9000.0000 4000.0000 1000.0000 1.91 GB

@majocha majocha marked this pull request as ready for review January 29, 2025 23:19
@majocha majocha requested a review from a team as a code owner January 29, 2025 23:19
@psfinaki
Copy link
Member

As with other OT changes, any Jaeger screenshots would be nice to have :)

@psfinaki
Copy link
Member

Hi @majocha would you like to add something else here or is it getting ready to merge?

@majocha
Copy link
Contributor Author

majocha commented Feb 10, 2025

@psfinaki I think it's ok. Don't know how useful this will be in general, but it does reduce allocations a bit.

@psfinaki
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@psfinaki psfinaki added the NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes label Feb 10, 2025
@psfinaki psfinaki merged commit 5d0812f into dotnet:main Feb 10, 2025
33 of 34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants