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

Add dotnet auto instrumentation #589

Closed
cartersocha opened this issue Nov 16, 2022 · 21 comments · Fixed by #1538
Closed

Add dotnet auto instrumentation #589

cartersocha opened this issue Nov 16, 2022 · 21 comments · Fixed by #1538
Labels
enhancement New feature or request telemetry issues that relate to the telemetry generated by the demo

Comments

@cartersocha
Copy link
Contributor

Dotnet should have non instrumentation library based auto instrumentation examples.

https://github.com/open-telemetry/opentelemetry-dotnet-instrumentation

@cartersocha cartersocha added the enhancement New feature or request label Nov 16, 2022
@cartersocha
Copy link
Contributor Author

@rajkumar-rangaraj, @reyang thoughts on potentially including this?

@reyang
Copy link
Member

reyang commented Nov 16, 2022

@open-telemetry/dotnet-instrumentation-maintainers

@reyang
Copy link
Member

reyang commented Nov 16, 2022

My couple cents:

  1. We should avoid using any undefined/misleading term in this demo project - across the docs, code and communication, e.g. "agent". https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/glossary.md
  2. We probably need to define "what's the default set of experiences/services/containers", to avoid this demo project becoming too big/complex/slow for average machines to run.
  3. .NET auto-instrumentation is currently at Beta, I can imagine potential breaking changes, I suggest getting feedback from @open-telemetry/dotnet-instrumentation-maintainers to see if it's a good time or we should defer this to RC or Stable.

@Kielek
Copy link
Contributor

Kielek commented Nov 16, 2022

SIG Auto Instrumentation .NET meeting notes: if we are able to use .NET Framework 4.6.2+/NET 6/7 we should be fine to include it shortly. We expect 0.5.0 release which will be supporting all modern .NET version (including .NET 7).

@pellared
Copy link
Member

Please do not call it "agent".

The upcoming 0.5.0 release will contain many changes in the configuration settings to avoid the possibility of possible breaking changes in future. We will do the best to describe potential problems when upgrading to newer versions in the GitHub Releases and CHANGELOG.md and troubleshooting.md.

Based on the existing feedback we get I would say that it is "production quality".

@cartersocha cartersocha changed the title Add agent based dotnet auto instrumentation Add dotnet auto instrumentation Nov 16, 2022
@pellared
Copy link
Member

0.5.0 is released.

CC @carlosalberto

@austinlparker austinlparker added the telemetry issues that relate to the telemetry generated by the demo label Feb 6, 2023
@austinlparker
Copy link
Member

Questions:

  • Is the auto-instrumentation the 'preferred' method of .NET instrumentation?

If yes, then we should replace the library instrumentation with it. If no...

  • Should we create a new service to demonstrate the auto-instrumentation, or can we modify the existing service to demonstrate both?

@cartersocha
Copy link
Contributor Author

They’re in beta rn so preferred might not be the choice today. @rajkumar-rangaraj thoughts?

@rajkumar-rangaraj
Copy link

.NET Auto-Instrumentation 0.5.0 is the first production-ready (non-beta) release. It is not stable yet. .NET Auto-Instrumentation could be used demonstrate a new service.

@Kielek
Copy link
Contributor

Kielek commented Apr 28, 2023

I think that we need some brand new service for this purposes. I would like to avoid mixing library instrumentation with auto-instrumentation. It is not typical usage.

Do you have any proposal how to extend this https://opentelemetry.io/docs/demo/architecture/?

@pellared
Copy link
Member

pellared commented Apr 28, 2023

I think that we need some brand new service for this purposes. I would like to avoid mixing library instrumentation with auto-instrumentation. It is not typical usage.

Do you have any proposal how to extend this https://opentelemetry.io/docs/demo/architecture/?

There are 3 services written in Go. See: https://opentelemetry.io/docs/demo/service-table/. Maybe we can rewrite one in .NET and use .NET Auto-Instrumentation there?

Is the auto-instrumentation the 'preferred' method of .NET instrumentation?

There is no strong preference. As far as I see, usually devs prefer the manual/SDK way and SREs prefer auto-instrumentation.

@cartersocha
Copy link
Contributor Author

The accounting service is essentially a placeholder. We’d more than welcome a rewrite of it. We are trying to avoid adding more services than we currently have due to resource requirements

@martinjt
Copy link
Member

My two cents is that Auto-instrumentation is not the recommended way, in pretty much any language. If you have access to the code, you should use the method that gives you most control. Once we have the configuration specifications sorted, it shouldn't matter from the outside which is what matters.

Having a service that is auto-instrumented as well as not is something I could get behind to provide both examples, however, not providing a manual instrumentation in the demo would be something I would be against.

I'd also agree with Reiley in that the demo is getting big, and adding essentially a duplication of all languages in an auto-instrumented way would bloat the project for no real benefit in my opinion.

The idea of auto-instrumentation is instrumenting a project that you don't have access to the code for. For that, I'd much prefer to take an existing Microservices based demo and add the operator, rather than use the OpenTelemetry demo for that as it would provide the right sort of message "You can add the operator to any solution and it will "just work" without changing code".

@austinlparker
Copy link
Member

If it wouldn't cause the code to be too messy I'd imagine we could do some sort of conditional enablement of the SDK and have a run-time flag to use auto-instrumentation or library instrumentation.

@cartersocha
Copy link
Contributor Author

@martinjt an existing manually instrumented dotnet service exists already, it is not being proposed we replace it or mix instrumentations, we would most likely replace an existing Go service to not increase overall demo size, and many SREs may not have access to the code as you mentioned but also not run on kubernetes.

If we wanted to use the operator in the kubernetes version that is fine and would be great to have. But the regular docker version needs auto instrumentation examples too and the point of the demo is to show how community assets, like dotnet auto instrumentation, work in the “wild”. This isn’t an instrumentation efficiency demo but a showcase

@austinlparker
Copy link
Member

Closing this as part of issue cleanup. If there's anything that needs to be done, please create a new issue.

@pellared
Copy link
Member

@austinlparker Can you please reopen it? I do not think creating a new one is a good idea as we would lose the discussion. I think the .NET Automatic Instrumentation SIG may try to address it before KubeCon EU 2024.

CC @open-telemetry/dotnet-instrumentation-approvers

@austinlparker austinlparker reopened this Feb 19, 2024
@austinlparker
Copy link
Member

No problem. There's a few places I think would be ideal for this; I might end up putting in a spike together this week for a .NET service that could use the zero-code instrumentation.

@Kielek
Copy link
Contributor

Kielek commented Feb 19, 2024

@austinlparker, the main blocker for introducing auto-instrumentation is lack of support for ARM64 platform. @RassK is working to cover this in open-telemetry/opentelemetry-dotnet-instrumentation#3277.

@austinlparker
Copy link
Member

Cool, do you think this is something that will exist by KubeCon?

@Kielek
Copy link
Contributor

Kielek commented Feb 19, 2024

I hope, but please do not treat it as a hard comitment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request telemetry issues that relate to the telemetry generated by the demo
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants