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

Fix leftovers after renaming profiler #84

Merged
merged 1 commit into from
Mar 11, 2021

Conversation

dszmigielski
Copy link
Contributor

@dszmigielski dszmigielski commented Mar 11, 2021

There were some leftovers after series of renaming PRs that we found out were not changed (some of them are new from upstream merge). So here they are.

@dszmigielski dszmigielski requested a review from a team as a code owner March 11, 2021 10:40
@dszmigielski dszmigielski reopened this Mar 11, 2021
@@ -2653,7 +2653,7 @@ void CorProfiler::GetAssemblyAndSymbolsBytes(BYTE** pAssemblyArray, int* assembl
for(auto i = 0; i < imgCount; i++) {
const std::string name = std::string(_dyld_get_image_name(i));

if (name.rfind("Datadog.Trace.ClrProfiler.Native.dylib") != std::string::npos) {
if (name.rfind("OpenTelemetry.AutoInstrumentation.ClrProfiler.Native.dylib") != std::string::npos) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line not being changed caused macOS profiler to fail at the application start, crashing the application with weird error if the profiler name didn't match the one in if (and it didn't since we have changed it to OTEL name):

Unhandled exception. System.OverflowException: Arithmetic operation resulted in an overflow.
at DDVoidMethodType.DDVoidMethodCall()
at Microsoft.DotNet.Cli.Program.Main(String[] args)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does anybody have a clue why does it fail like that, and why do we have to hardcode the name of the output file in the code?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

@dszmigielski please create an issue for it

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.

Tested more or less together 😉

@pjanotti pjanotti merged commit c954322 into open-telemetry:main Mar 11, 2021
@dszmigielski dszmigielski deleted the so-dylib-rename branch March 23, 2021 14:22
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.

None yet

4 participants