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

Propose/Design the way of referencing/using VS OTel #10947

Open
JanKrivanek opened this issue Nov 7, 2024 · 9 comments · May be fixed by #11084 or #11175
Open

Propose/Design the way of referencing/using VS OTel #10947

JanKrivanek opened this issue Nov 7, 2024 · 9 comments · May be fixed by #11084 or #11175
Assignees
Labels
Cost:S Work that requires one engineer up to 1 week Priority:1 Work that is critical for the release, but we could probably ship without

Comments

@JanKrivanek
Copy link
Member

JanKrivanek commented Nov 7, 2024

Motivation

VS OTel collector and extensions require dependencies that are currently only VS specific (Microsoft.VisualStudio.OpenTelemetry.ClientExtensions, Microsoft.VisualStudio.OpenTelemetry.Collector). We need to start collector ourselves if we want to collect data from standalone process (msbuild.exe run).

Additionaly - We currently produce the MSBuild bits - including the msbuild.exe - from our GH repo, that is expected to be source-buildable.
The requirement technicaly applies only to the core version.
Also - we want runnable bootstrap - in order not to break our testing infra

Goal

Propose/Design the way of referencing/mounting VS OTel extensions and collector. The proposal should be approved by msbuild team + product construction team.

@JanKrivanek JanKrivanek changed the title Propose/Design the way of referencing VS OTel dependencies that are not 'source buildable' (conditional compilation or extract the mounting of collector to VS) - this should be approved by msbuild team + product construction team (JanP + JanK) Propose/Design the way of referencing/using VS OTel Nov 7, 2024
@JanKrivanek JanKrivanek added Priority:1 Work that is critical for the release, but we could probably ship without Cost:S Work that requires one engineer up to 1 week labels Nov 7, 2024
@JanKrivanek
Copy link
Member Author

Inspiration provided by @baronfel: https://github.com/baronfel/otel-startup-hook

@JanProvaznik
Copy link
Member

2 possible paths:

  1. add the telemetry to Microsoft.Build.Framework.dlls, this is explored in WIP telemetry [ do not merge] #11084
    a) OpenTelemetry part compiles in core and framework
    b) VS Telemetry part compiles only in framework
    c) Controlled by env vars if there is an otel var present it will start the local collector, should not start collector when in VS
    d) if there is an optout it should not start
  1. Ship another DLL in .NET Framework distribution of MSBuild that is then loaded in XMake or BuildManager at the start of the build
    a) add helper methods for easily creating .NET traces (System.Diagnostics.Activity) compatible with the VS OTel format in Microsoft.Build.Framework
    b) new DLL which is in Framework somehow hooked at the beginning (analogously to how core's Startups hook work) of a build and starts TracerProvider, starts Collector if MSBuild.exe
    c) document how to hook your own collector (activity names/prefixes,) and point to e.g. baronfel/otel-startup-hook with samples
    @JanKrivanek please add details here

@rainersigwald @baronfel thoughts?

@JanKrivanek
Copy link
Member Author

Proposal that we've been putting together with @JanProvaznik:

We'll instrument only leveraging System.Diagnostic primitives, plus add minimal pluggability - creating no bariers for SourceBuild nor for consuming the data via exporter of choice.

  • .NET Version of MSBuild
    • Exporting to our endpoints
      • Will remain unchanged for now (driven by sdk, data emited to System.Diagnoastic primitives won't be considered)
    • Customizable exporting to user endpoints
      • It'd be up to users to hook the telemetry exporter leveraging the Host Startup Hook.
      • The TraceName(s) will be documented in our doc
      • We'll point (in the same doc) to https://github.com/baronfel/otel-startup-hook as a reference
      • The doc will as well contain sample usage (setting of env vars)
  • NetFx version of MSBuild
    • Exporting to our endpoints
      • VS Telemetry exporter will be compiled-in only for NetFx, it'll be started only if MSBuild is not hosted in VS and if users didn't opted out (TBD: define exact conditions for this)
      • It'll be started in Xmake or BuildManager (whichever is called first)
    • Customizable exporting to user endpoints
      • Let's consider adding customizable startup hooking (e.g. MSBUILD_STARTUP_HOOKS analogy to DOTNET_STARTUP_HOOKS, maybe applicalbe even for API calls) - this way we'd enable custom exporting of data without need to stick to any specific collector/exporter
      • The exporting hooked by custom hook versus the VS hooked exporting should be independent and can happen at the same time (or none if opted out)

@JanKrivanek
Copy link
Member Author

@JanProvaznik HAHA - we've nicely jinxed on this 😄

@JanProvaznik
Copy link
Member

Summary from discussion:
There might be interest in the future from 1ES for hooking collector in MSBuild.exe

  • @JanKrivanek will look into if there is an already an existing implementation of Startup hooking for Framework
    • we can implement it ourselves using reflection if not found/unusable
      VS collector can be hardcoded with conditional compilation for now.

We can proceed to implementation

  • Be careful not to allocate metadata when activity would not be client side sampled.
    We push for collector being started in VS, we don't want to hook it ourselves when in VS, sampling both client and server side.

@JanKrivanek
Copy link
Member Author

As for .NET Fx hooking - we might try to leverage APPDOMAIN_MANAGER_TYPE https://www.rapid7.com/blog/post/2023/05/05/appdomain-manager-injection-new-techniques-for-red-teams/

@JanProvaznik
Copy link
Member

rapid7.com/blog/post/2023/05/05/appdomain-manager-injection-new-techniques-for-red-teams

Thanks, I investigated this and found it quite clunky.
You have to place the hook in the same spot as MSBuild.exe and this leads to assembly version conflicts (System.Memory) of MSBuild and the hook dlls.

@JanProvaznik
Copy link
Member

@JanProvaznik
Copy link
Member

JanProvaznik commented Dec 16, 2024

State of the investigation:
So for VS case: we create ActivitySource and Exporter (and add it to TracerProvider). VS has the collector (will have once it's implemented in January).
MSBuild.exe case: we create ActivitySource, Exporter, Collector - for prototype v1 include in Microsoft.Build.Framework, prototype v2 also enable hooking custom exporters/collectors via doublehook #11148
core case: only create Activity Source, people can hook with core hook https://github.com/baronfel/otel-startup-hook

@JanProvaznik JanProvaznik linked a pull request Dec 19, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cost:S Work that requires one engineer up to 1 week Priority:1 Work that is critical for the release, but we could probably ship without
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants