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

Continuous profiler - test application #3084

Merged

Conversation

Kielek
Copy link
Contributor

@Kielek Kielek commented Nov 9, 2023

Why

Towards #3074

What

Continuous profiler - test application for .NET. It covers:

  • Languages:
    • C#
    • VB
    • F#
    • C++
  • Calls from Managed Code -> Native Code -> Managed Code.
  • Generic classes and methods
  • Non-generic classes and methods
  • Custom types
    • classes
    • structs
  • Built in types
    • classes
    • structs
  • Dynamic methods
  • Nested classes
  • Modified build pipeline

.NET Framework is not needed, as used solution requires .NET Core 3.1/.NET 5.0+ - which means .NET 6.0 for us.

Test application will be used to generate stack traces to test continuous profiler.

Tests

CI + Added test to check if the application is executed without issues.

Checklist

  • [ ] CHANGELOG.md is updated.
  • [ ] Documentation is updated.
  • New features are covered by tests.

Notes

Merging this does not affects our plans for 1.2.0 release. It even proofs that application including VB/F#/C++ is still working.

Co-authored-by: Mateusz Łach <[email protected]>
Co-authored-by: Robert Pająk <[email protected]>
@Kielek Kielek requested a review from a team November 9, 2023 13:00
@rajkumar-rangaraj
Copy link
Contributor

Can we hold on to this one until we discuss and get consensus from all of the maintainers?

Copy link
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

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

Co-authored-by: Robert Pająk <[email protected]>
@Kielek
Copy link
Contributor Author

Kielek commented Nov 10, 2023

Can we hold on to this one until we discuss and get consensus from all of the maintainers?

Sure, it is not first time when we rise this topic and the plan was briefly approved few month ago :)

I will wait for @zacharycmontoya, @nrcventura and you before merging this PR.

Copy link
Member

@nrcventura nrcventura left a comment

Choose a reason for hiding this comment

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

I think that having the test apps in place is a good first step.

I think what still needs to be determined is where to place, how to identify, and how to enable the experimental features such as continuous profiling.

@Kielek Kielek force-pushed the continuous-profiler-test-application branch from 8a76780 to 2b88ca4 Compare November 24, 2023 08:44
@Kielek Kielek merged commit 9b260c6 into open-telemetry:main Dec 1, 2023
31 checks passed
@Kielek Kielek deleted the continuous-profiler-test-application branch December 1, 2023 18:37
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.

7 participants