feat: store tracer configuration in an in-memory file #3171

dmehala · 2025-02-10T14:22:07Z

What does this PR do?

This is PR store the tracer configuration in an memfd file.

Motivation

Note to reviewers

Sorry, I am not well-versed in Go. Please feel free to take over the pull request to ensure it aligns more closely with idiomatic practices and fits better within the codebase.

Reviewer's Checklist

Changed code has unit tests for its functionality at or near 100% coverage.
System-Tests covering this feature have been added and enabled with the va.b.c-dev version tag.
There is a benchmark for any new code, or changes to existing code.
If this interacts with the agent in a new way, a system test has been added.
Add an appropriate team label so this PR gets put in the right place for the release notes.
Non-trivial go.mod changes, e.g. adding new modules, are reviewed by @DataDog/dd-trace-go-guild.
For internal contributors, a matching PR should be created to the v2-dev branch and reviewed by @DataDog/apm-go.

Unsure? Have a question? Request a review!

datadog-datadog-prod-us1 · 2025-02-10T14:28:07Z

Datadog Report

Branch report: dmehala/feat/process-discovery
Commit report: 09fcf6f
Test service: dd-trace-go

✅ 0 Failed, 5386 Passed, 73 Skipped, 2m 48.75s Total Time

pr-commenter · 2025-02-10T15:03:27Z

Benchmarks

Benchmark execution time: 2025-03-07 14:03:29

Comparing candidate commit b4c3297 in PR branch dmehala/feat/process-discovery with baseline commit 0e4174c in branch main.

Found 0 performance improvements and 1 performance regressions! Performance is the same for 59 metrics, 2 unstable metrics.

scenario:BenchmarkStartRequestSpan-24

🟥 execution_time [+58.851ns; +60.109ns] or [+12.936%; +13.213%]

dmehala · 2025-02-11T14:57:51Z

ddtrace/tracer/tracer.go

+
+	metadata := TracerMetadata{
+		SchemaVersion:      1,
+		RuntimeId:          "TBD", ///< Where to get it?


I need guidance here on how to get this the runtimeID.

globalconfig.RuntimeID()

addressed in 7bb75db

dmehala · 2025-02-11T14:57:57Z

ddtrace/tracer/tracer.go

+		SchemaVersion:      1,
+		RuntimeId:          "TBD", ///< Where to get it?
+		Language:           "golang",
+		Version:            "1.2.3",


For the tracer's version, it's in internal/version.Tag.

addressed in 7bb75db

kakkoyun

Could you provide some context? What's the goal and intended use case for storing the config in an in-memory file?

dmehala · 2025-02-12T15:40:19Z

@kakkoyun Process Discovery.

darccio

We should implement testing for this covering the different situations. You can use runtime.GOOS == "linux" to run the test only in Linux.

darccio · 2025-02-18T08:55:25Z

ddtrace/tracer/tracer.go

+
+	metadata := TracerMetadata{
+		SchemaVersion:      1,
+		RuntimeId:          "TBD", ///< Where to get it?


globalconfig.RuntimeID()

darccio · 2025-02-18T08:57:56Z

ddtrace/tracer/tracer.go

+		SchemaVersion:      1,
+		RuntimeId:          "TBD", ///< Where to get it?
+		Language:           "golang",
+		Version:            "1.2.3",


For the tracer's version, it's in internal/version.Tag.

ddtrace/tracer/tracer.go

internal/inmemoryfilelinux.go

dmehala · 2025-02-21T13:28:11Z

We should implement testing for this covering the different situations. You can use runtime.GOOS == "linux" to run the test only in Linux.

Unless I mock the syscall, I am afraid this not very testable. Do you have anything else in mind @darccio?

darccio · 2025-02-24T09:16:05Z

@dmehala Why do you need to mock the call?

dmehala · 2025-02-26T22:44:30Z

@darccio Well, if the goal is to check that the memfd is created and contains the correct value, we already have a system test for that. Would that be enough, or are you looking for something more?

darccio · 2025-03-07T11:26:22Z

@dmehala If we have a system test for this, let's go forward.

felixge · 2025-04-02T21:02:29Z

ddtrace/tracer/tracer.go

+	// store the configuration in an in-memory file, allowing it to be read to
+	// determine if the process is instrumented with a tracer and to retrive
+	// relevant tracing information.
+	storeConfig(t.config)


👋 I came across the code in this PR (and the RFC for it) today. It's really cool. I didn't know about the memfd syscalls - I might have some use cases for it in the future as well!

Anyway, I'm commenting b/c I think there is a slight problem here. This code is only considering the tracer product. I think there is a problem when a customer is only enabling the profiler, but not the tracer. This does happen sometimes in the real world AFAIK.

feat: store tracer configuration in an in-memory file

f6a9d79

dmehala force-pushed the dmehala/feat/process-discovery branch from 9ba3247 to f6a9d79 Compare February 11, 2025 14:40

dmehala marked this pull request as ready for review February 11, 2025 14:43

dmehala requested review from a team as code owners February 11, 2025 14:43

dmehala commented Feb 11, 2025

View reviewed changes

kakkoyun reviewed Feb 12, 2025

View reviewed changes

dmehala requested a review from darccio February 13, 2025 16:42

darccio requested changes Feb 18, 2025

View reviewed changes

kakkoyun reviewed Feb 19, 2025

View reviewed changes

ddtrace/tracer/tracer.go Outdated Show resolved Hide resolved

internal/inmemoryfilelinux.go Outdated Show resolved Hide resolved

code review

7bb75db

dmehala requested review from darccio and kakkoyun February 21, 2025 11:20

Merge branch 'main' into dmehala/feat/process-discovery

589e751

Merge branch 'main' into dmehala/feat/process-discovery

c305848

darccio approved these changes Mar 7, 2025

View reviewed changes

dmehala and others added 2 commits March 7, 2025 14:06

Merge branch 'main' into dmehala/feat/process-discovery

f97a173

Merge branch 'main' into dmehala/feat/process-discovery

b4c3297

kakkoyun approved these changes Mar 7, 2025

View reviewed changes

darccio merged commit 4800665 into main Mar 7, 2025
176 checks passed

darccio deleted the dmehala/feat/process-discovery branch March 7, 2025 14:18

felixge reviewed Apr 2, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: store tracer configuration in an in-memory file #3171

feat: store tracer configuration in an in-memory file #3171

dmehala commented Feb 10, 2025 •

edited

Loading

datadog-datadog-prod-us1 bot commented Feb 10, 2025 •

edited

Loading

pr-commenter bot commented Feb 10, 2025 •

edited

Loading

dmehala Feb 11, 2025

darccio Feb 18, 2025

dmehala Feb 19, 2025

dmehala Feb 11, 2025

darccio Feb 18, 2025

dmehala Feb 19, 2025

kakkoyun left a comment

dmehala commented Feb 12, 2025

darccio left a comment

darccio Feb 18, 2025

darccio Feb 18, 2025

dmehala commented Feb 21, 2025 •

edited

Loading

darccio commented Feb 24, 2025

dmehala commented Feb 26, 2025

darccio commented Mar 7, 2025

felixge Apr 2, 2025

feat: store tracer configuration in an in-memory file #3171

feat: store tracer configuration in an in-memory file #3171

Conversation

dmehala commented Feb 10, 2025 • edited Loading

What does this PR do?

Motivation

Note to reviewers

Reviewer's Checklist

datadog-datadog-prod-us1 bot commented Feb 10, 2025 • edited Loading

Datadog Report

pr-commenter bot commented Feb 10, 2025 • edited Loading

Benchmarks

scenario:BenchmarkStartRequestSpan-24

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kakkoyun left a comment

Choose a reason for hiding this comment

dmehala commented Feb 12, 2025

darccio left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmehala commented Feb 21, 2025 • edited Loading

darccio commented Feb 24, 2025

dmehala commented Feb 26, 2025

darccio commented Mar 7, 2025

Choose a reason for hiding this comment

dmehala commented Feb 10, 2025 •

edited

Loading

datadog-datadog-prod-us1 bot commented Feb 10, 2025 •

edited

Loading

pr-commenter bot commented Feb 10, 2025 •

edited

Loading

dmehala commented Feb 21, 2025 •

edited

Loading