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

Support DotNet auto-instrumentation #976

Conversation

avadhut123pisal
Copy link
Contributor

@avadhut123pisal avadhut123pisal commented Jul 13, 2022

This PR adds a support for auto-instrumentation of the .Net applications.
Currently, dot net auto-instrumentation image is using the release opentelemetry-dotnet-instrumentation v0.2.0-beta.1.

Following environment variables are injected to the application container to enable the auto-instrumentation.

DOTNET_ADDITIONAL_DEPS=%InstallationLocation%/AdditionalDeps
DOTNET_SHARED_STORE=%InstallationLocation%/store
DOTNET_STARTUP_HOOKS=%InstallationLocation%/netcoreapp3.1/OpenTelemetry.AutoInstrumentation.StartupHook.dll

Resolves #756

@avadhut123pisal avadhut123pisal requested a review from a team July 13, 2022 11:55
@avadhut123pisal avadhut123pisal changed the title Support DotNet autoinstrumentation Support DotNet auto-instrumentation Jul 13, 2022
@avadhut123pisal
Copy link
Contributor Author

e2e test case is failing for newly added dot-net auto-instrumentation with the error in pulling the docker image ghcr.io/open-telemetry/opentelemetry-operator/autoinstrumentation-dotnet:0.0.1-alpha. But, until the review gets completed and it is merged, this image will not be available. So, how to deal with this ?

@pavolloffay
Copy link
Member

But, until the review gets completed and it is merged, this image will not be available. So, how to deal with this ?

Please open a separate PR just to build and publish the image

@avadhut123pisal
Copy link
Contributor Author

But, until the review gets completed and it is merged, this image will not be available. So, how to deal with this ?

Please open a separate PR just to build and publish the image

I have opened a separate PR to build and publish the image #989

@pavolloffay
Copy link
Member

what is the binary file .DS_Store in this PR?

argument: "0.25"
dotnet:
env:
- name: OTEL_DOTNET_AUTO_TRACES_ENABLED_INSTRUMENTATIONS
Copy link
Member

Choose a reason for hiding this comment

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

is this required? What happens if the variable is empty?

Copy link
Contributor Author

@avadhut123pisal avadhut123pisal Jul 25, 2022

Choose a reason for hiding this comment

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

The PR #932 Enable all trace instrumentations by default (simplified) for enabling the all trace instrumentation by default was merged few days back. It is not available as a part of opentelemetry-dotnet-instrumentation v0.2.0-beta.1.

So, OTEL_DOTNET_AUTO_TRACES_ENABLED_INSTRUMENTATIONS is required to be specified for the release opentelemetry-dotnet-instrumentation v0.2.0-beta.1.

If it is empty, then applications runs but trace instrumentation does not work.

Copy link
Member

Choose a reason for hiding this comment

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

Will this change in the future? If no the operator should set this variable on the behalf of the user.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously, this env variable's name was OTEL_DOTNET_TRACER_INSTRUMENTATIONS . In v0.2.0-beta.1, it is renamed to OTEL_DOTNET_AUTO_TRACES_ENABLED_INSTRUMENTATIONS .

As of now, we can set this env value to AspNet,HttpClient,SqlClient but in the next release of the dotnet auto-instrumentation Npgsql, MySqlData will be added.

Copy link
Member

Choose a reason for hiding this comment

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

We should definitely document this in the readme and the operator could set the variable (if it is not set) in the defaulting webhook

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I have pushed the changes.

@avadhut123pisal
Copy link
Contributor Author

what is the binary file .DS_Store in this PR?

My bad. This file is not required. I will delete it and commit.

@pavolloffay pavolloffay merged commit 7ce297d into open-telemetry:main Jul 26, 2022
ItielOlenick pushed a commit to ItielOlenick/opentelemetry-operator that referenced this pull request May 1, 2024
* Adds support for dotnet auto-instrumentation

* changes default image for testing

* adds dotnet auto-instrumentation to the ldflags in makefile

* changes in Makefile

* fixes issue in makefile

* fixes issue in dockerfile

* passes dotnet sample app image to e2e test

* fixes version in the dot net autoinstrumentation version file

* adds testing changes for dot-net-autoinstrumentation

* fixes annotation

* downloads dot-net release from github

* removes git push condition check

* removes git push condition check

* changes repo to logicmonitor

* adds dontnet auto-instrumentation related details in readme

* adds docker image layer caching in github action

* triggers action on pull request

* adds e2e test cases for dotnet instrumentation

* fixes version for dotnet instrumentation

* version changes

* restores clusterversion file

* restores changes in Makefile

* minor change

* Adds configuration details to dockerfile

* updates version of dotnet auto-instrumentation

* changes env var name from OTEL_DOTNET_TRACER_INSTRUMENTATIONS to OTEL_DOTNET_AUTO_TRACES_ENABLED_INSTRUMENTATIONS

* removes java related env variables

* fixes e2e test cases

* fixes e2e test case

* deletes .DS_Store file

* sets OTEL_DOTNET_AUTO_TRACES_ENABLED_INSTRUMENTATIONS to default value

* fixes e2e test case

* fixes e2e test case
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.

Add support for dotnet C# auto-instrumentation
2 participants