Add SourceLink to native Windows DLL

[martincostello]

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.

[martincostello]

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]

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.

[martincostello]

how to correctly build .snupkg file for the native part

Short answer is "no idea" 😄

[martincostello]

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]

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]

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]

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:

opentelemetry-dotnet-instrumentation/build/Build.NuGet.Steps.cs

Lines 138 to 142 in 45a3b23

	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:

opentelemetry-dotnet-instrumentation/nuget/OpenTelemetry.AutoInstrumentation.Runtime.Native/OpenTelemetry.AutoInstrumentation.Runtime.Native.nuspec

Line 25 in 45a3b23

<file src="**\*" target="\" />

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.

[Kielek]

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]

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

This isn't the case:

opentelemetry-dotnet-instrumentation/test/IntegrationTests/BuildTests.DistributionStructure_windows.verified.txt

Lines 9 to 16 in 45a3b23

	\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,

opentelemetry-dotnet-instrumentation/test/IntegrationTests/BuildTests.DistributionStructure_windows.verified.txt

Line 62 in 45a3b23

\netfx\OpenTelemetry.AutoInstrumentation.pdb,

[Copilot]

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]

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

[zacharycmontoya]

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]

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).