Skip to content

Add SourceLink to native Windows DLL#4774

Merged
Kielek merged 2 commits intoopen-telemetry:mainfrom
martincostello:native-source-link
Jan 19, 2026
Merged

Add SourceLink to native Windows DLL#4774
Kielek merged 2 commits intoopen-telemetry:mainfrom
martincostello:native-source-link

Conversation

@martincostello
Copy link
Member

Why

Fixes BinSkim warning I saw while working on #4768:

OpenTelemetry.AutoInstrumentation.Native.dll: warning BA2027: The PDB for 'OpenTelemetry.AutoInstrumentation.Native.dll' does not contain SourceLink information, compromising frictionless source-driven debugging and increasing latency of security response.
Enable SourceLink by configuring necessary project properties and adding a package reference for your source control provider.
See https://aka.ms/sourcelink for more information.

What

Configure SourceLink for the native Windows DLL. Based on the example implemented provided here.

This appears to be an MSVC-specific feature, so there isn't a macOS or Linux equivalent as far as I'm aware.

Tests

Manually ran BinSkim on a locally compiled DLL and the warning isn't present.

Example content of the generated source_link.json file:

{"documents": { "D:\\Coding\\open-telemetry\\opentelemetry-dotnet-instrumentation\\*" : "https://raw.githubusercontent.com/martincostello/opentelemetry-dotnet-instrumentation/8274215e86868e90c181aed15e97535c33e70cb3/*" }}

Will also verify against the CI build once completed.

Checklist

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

Configure SourceLink for the native Windows DLL.
@martincostello
Copy link
Member Author

Will also verify against the CI build once completed.

No warning, but there's no PDB in the output to verify.

Should the PDB/symbols for the binary files be output in the artifacts too?

@Kielek
Copy link
Member

Kielek commented Jan 16, 2026

Will also verify against the CI build once completed.

No warning, but there's no PDB in the output to verify.

Should the PDB/symbols for the binary files be output in the artifacts too?

I am not against, one more thing to check, how to correctly build .snupkg file for the native part. I am not sure what are recommendation/support for this.

Include the PDBs for the Windows native DLLs in the build output.
@martincostello
Copy link
Member Author

how to correctly build .snupkg file for the native part

Short answer is "no idea" 😄

@martincostello
Copy link
Member Author

Potentially one way to do it is to just include the symbols in the package itself via OpenTelemetry.AutoInstrumentation.Runtime.Native.nuspec. I'll read into it to see if there's an easy way to create symbol packages when there's only a .nuspec file.

@martincostello
Copy link
Member Author

From NuGet.org symbol package constraints:

Note

Native projects, such as C++ projects, produce Windows PDBs instead of Portable PDBs. These are not supported by NuGet.org's symbol server. Please use Legacy Symbol Packages instead.

Will look at Legacy Symbol Packages next...

@martincostello
Copy link
Member Author

No warning, but there's no PDB in the output to verify.

Symbols included (and found) now, and there's no warning related to SourceLink:

OpenTelemetry.AutoInstrumentation.Native.dll: warning BA2024: 'OpenTelemetry.AutoInstrumentation.Native.dll' was compiled with one or more modules that do not enable code generation mitigations for speculative execution side-channel attack (Spectre) vulnerabilities.
Spectre attacks can compromise hardware-based isolation, allowing non-privileged users to retrieve potentially sensitive data from the CPU cache.
To resolve this issue, ensure that all modules compiled into the binary are compiled with /Qspectre switch on cl.exe command-line.
You may need to install the 'C++ spectre-mitigated libs' component from the Visual Studio installer if you observe violations against C runtime libraries such as libcmt.lib, libvcruntime.lib, etc.
For VC projects use SpectreMitigation property with 'Spectre' value.
When using older MSVC versions pass /d2guardspecload in cases where your compiler supports this switch and it is not possible to update to a toolset that supports /Qspectre.
This warning should be addressed for code that operates on data that crosses a trust boundary and that can affect execution, such as parsing untrusted file inputs or processing query strings of a web request.
For mitigation to be effective, all dlls and libs that are part of an exe, should be compiled with /QSpectre, so decision should be made for the whole process.
Can cause some perf degradations, so should not be used when not needed.

Done. 1 files scanned.
Analysis completed successfully.

@martincostello
Copy link
Member Author

Will look at Legacy Symbol Packages next...

I can produce .symbols.nupkg files for OpenTelemetry.AutoInstrumentation and OpenTelemetry.AutoInstrumentation.Runtime.Native by adding the following here:

NuGetTasks.NuGetPack(s => s
.SetTargetPath(nuspecProject)
.SetConfiguration(BuildConfiguration)
.SetProperties(nuspecCommonProperties)
.SetOutputDirectory(NuGetArtifactsDirectory));

- .SetOutputDirectory(NuGetArtifactsDirectory));
+ .SetOutputDirectory(NuGetArtifactsDirectory)
+ .EnableSymbols());

Other packages still produce .snupkg files.

However, the contents are effectively identical because the PDBs are already included in the NuGet packages anyway:

With a change like below, the PDBs are missing from both packages:

- <file src="**\*" target="\" />
+ <file src="**\*.dll" target="\" />
+ <file src="**\*.dylib" target="\" />
+ <file src="**\*.so" target="\" />

There doesn't appear to be a way to make <file> elements conditional.

Options seem to be:

  1. Leave as-is
  2. Create the .symbols.nupkg files anyway
  3. Exclude the .pdb files from the NuGet package.

I would go with 1 or 2.

Copy link
Member

@Kielek Kielek 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 the symbols should be considered as a separate artifacts (and included into release artifacts).
Typically, it is not needed for normal use-cases, but can be valuable for debugging purposes.

For the same reasons zip files does not include pdb files for the managed code.

@martincostello
Copy link
Member Author

For the same reasons zip files does not include pdb files for the managed code.

This isn't the case:

\net\OpenTelemetry.AutoInstrumentation.AspNetCoreBootstrapper.dll,
\net\OpenTelemetry.AutoInstrumentation.AspNetCoreBootstrapper.pdb,
\net\OpenTelemetry.AutoInstrumentation.Loader.dll,
\net\OpenTelemetry.AutoInstrumentation.Loader.pdb,
\net\OpenTelemetry.AutoInstrumentation.StartupHook.dll,
\net\OpenTelemetry.AutoInstrumentation.StartupHook.pdb,
\net\OpenTelemetry.AutoInstrumentation.dll,
\net\OpenTelemetry.AutoInstrumentation.pdb,

@martincostello martincostello marked this pull request as ready for review January 16, 2026 13:17
@martincostello martincostello requested a review from a team as a code owner January 16, 2026 13:17
Copilot AI review requested due to automatic review settings January 16, 2026 13:17
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request adds SourceLink support to the native Windows DLL to address a BinSkim warning about missing SourceLink information in the PDB. The implementation configures MSVC's /SOURCELINK linker option and generates a source_link.json file during the build process that maps source file paths to their corresponding GitHub URLs.

Changes:

  • Configured SourceLink for the native Windows DLL by adding an MSBuild target that generates the source_link.json file
  • Updated the build process to copy PDB files alongside DLL files for both x64 and x86 architectures
  • Updated the test verification file to include the newly-distributed PDB files

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
src/OpenTelemetry.AutoInstrumentation.Native/OpenTelemetry.AutoInstrumentation.Native.DLL.vcxproj Adds SourceLink configuration, linker options, and MSBuild target to generate source_link.json from git repository information
build/Build.Steps.Windows.cs Updates build script to copy both DLL and PDB files to distribution directory
test/IntegrationTests/BuildTests.DistributionStructure_windows.verified.txt Updates expected file list to include PDB files for both x64 and x86 architectures

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@martincostello
Copy link
Member Author

ZIP artifact sizes:

Name Before After % Delta
bin-alpine-arm64 4.09 MB 4.09 MB -
bin-alpine-x64 4.14 MB 4.14 MB -
bin-macos-14 3.28 MB 3.28 MB -
bin-ubuntu-22.04 4.02 MB 4.02 MB -
bin-ubuntu-22.04-arm 4 MB 4 MB -
bin-ubuntu1604-native 1.24 MB 1.24 MB -
bin-windows-2022 8.36 MB 17 MB x2.0
bin-nuget-packages 7.5 MB 16 MB x2.1

@Kielek Kielek merged commit b70fab8 into open-telemetry:main Jan 19, 2026
95 of 97 checks passed
@martincostello martincostello deleted the native-source-link branch January 19, 2026 07:15
@zacharycmontoya
Copy link
Contributor

I just got around to looking at this PR. By including the PDB's we're doubling the size of the artifacts. Is that required for this to work or can we separate out the artifacts as Piotr initially mentioned?

@martincostello
Copy link
Member Author

Whilst they are 2x, the absolute difference isn't much.

As noted here #4774 (comment), PDBs are already included in the artifacts, so you could argue including extra ones is more correct.

If you want to optimise for size, then IMHO that should be done comprehensively as a separate thing (i.e. to have separate "debug" artifacts).

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.

5 participants

Comments