Skip to content

fix(serverless-init): attach per-instance tag to MicroVM enhanced usage metric - #53230

Merged
litianningdatadog merged 2 commits into
tianning.li/microvm-07-06-trace-tag-race-fixfrom
tianning.li/microvm-07-07-enhanced-usage-metric-instance-tag
Jul 23, 2026
Merged

fix(serverless-init): attach per-instance tag to MicroVM enhanced usage metric#53230
litianningdatadog merged 2 commits into
tianning.li/microvm-07-06-trace-tag-race-fixfrom
tianning.li/microvm-07-07-enhanced-usage-metric-instance-tag

Conversation

@litianningdatadog

Copy link
Copy Markdown

Motivation

Codex flagged this on PR #53093 (review comment): MicroVM's enhanced usage metric (aws.lambda.microvm.instance) is emitted with the same static tag set for every MicroVM booted from a given image.

MicroVM.GetEnhancedMetricTags computes the Usage tag set once at startup — before the MicroVM's instance ID is known, since the platform only reveals it via the /run lifecycle hook — and nothing ever updates it afterward. Under normal autoscaling, with multiple concurrent MicroVMs running from the same image, their periodic usage samples are indistinguishable from one another: instance-level usage collapses into a single series.

Solution

The obvious fix — mutate the frozen tag set on ServerlessMetricAgent once /run reveals the instance ID, mirroring the existing LogsTagSetter/TraceTagSetter pattern — turned out to require a lock/atomic on ServerlessMetricAgent. That struct is passed by value into CloudService.Shutdown across every cloud service (AppService, CloudRun, CloudRunJobs, ContainerApp, LocalService, MicroVM), so adding any lock-bearing field trips go vet's copylocks check everywhere, not just for MicroVM.

Instead, this attaches the tag at the point of emission rather than mutating shared state:

  • lifecycle.Server already tracks the instance ID race-free (instanceID *atomic.String, captured in handleRun). Added InstanceID(), a nil-safe accessor, so callers outside the lifecycle package can read it.
  • MicroVM.CurrentUsageMetricTags() turns that into an "instance:<id>" tag, or nil before /run has fired.
  • enhancedmetrics.Collector gained an optional usageMetricTagsFunc, invoked on every collection tick and passed straight through the existing AddEnhancedUsageMetric(..., extraTags ...string) parameter — no new shared mutable state needed.
  • main.go duck-types cloudService against a local usageMetricTagProvider interface to wire this hook. Every other cloud service doesn't implement it, so usageMetricTagsFunc stays nil for them and their usage metrics are unaffected.

This keeps the change scoped to MicroVM plus small, nil-safe, additive plumbing in the shared collector — no other cloud service, ServerlessMetricAgent, or the Shutdown interface needed to change.

Test plan

  • dda inv test --targets=./cmd/serverless-init/... — all 281 tests pass (12 new: InstanceID() nil/before/after /run, CurrentUsageMetricTags() nil-server/before-run/after-run end-to-end, collector dynamic-tag forwarding + nil-func regression guard, NewCollector wiring, and the usageMetricTagProvider type-assertion pinning *MicroVM in vs. every other cloud service out)
  • gofmt -l clean on all changed files
  • dda inv linter.go --targets=./cmd/serverless-init/... — 0 issues
  • cmd/serverless-init/enhanced-metrics/collector.go/collector_test.go carry a pre-existing //go:build linux tag; this session ran on macOS without a Linux cross-toolchain, so those specific new tests are syntax-checked (gofmt -e) and reviewed but not yet executed — first real run will be in Linux CI

🤖 Generated with Claude Code

@litianningdatadog
litianningdatadog requested a review from a team as a code owner July 6, 2026 04:09
@datadog-datadog-prod-us1

This comment has been minimized.

@dd-octo-sts

dd-octo-sts Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Static quality checks

✅ Please find below the results from static quality gates
Comparison made with ancestor 1f54e07
📊 Static Quality Gates Dashboard
🔗 SQG Job

33 successful checks with minimal change (< 2 KiB)
Quality gate Current Size
agent_deb_amd64 750.599 MiB
agent_deb_amd64_fips 705.387 MiB
agent_heroku_amd64 307.504 MiB
agent_msi 639.379 MiB
agent_rpm_amd64 750.583 MiB
agent_rpm_amd64_fips 705.370 MiB
agent_rpm_arm64 727.261 MiB
agent_rpm_arm64_fips 685.184 MiB
agent_suse_amd64 750.583 MiB
agent_suse_amd64_fips 705.370 MiB
agent_suse_arm64 727.261 MiB
agent_suse_arm64_fips 685.184 MiB
docker_agent_amd64 809.275 MiB
docker_agent_arm64 810.989 MiB
docker_agent_jmx_amd64 1000.173 MiB
docker_agent_jmx_arm64 990.539 MiB
docker_cluster_agent_amd64 209.843 MiB
docker_cluster_agent_arm64 222.890 MiB
docker_cws_instrumentation_amd64 7.439 MiB
docker_cws_instrumentation_arm64 6.877 MiB
docker_dogstatsd_amd64 39.228 MiB
docker_dogstatsd_arm64 37.368 MiB
docker_host_profiler_amd64 303.231 MiB
docker_host_profiler_arm64 314.740 MiB
dogstatsd_deb_amd64 29.968 MiB
dogstatsd_deb_arm64 28.012 MiB
dogstatsd_rpm_amd64 29.968 MiB
dogstatsd_suse_amd64 29.968 MiB
iot_agent_deb_amd64 46.141 MiB
iot_agent_deb_arm64 42.833 MiB
iot_agent_deb_armhf 43.601 MiB
iot_agent_rpm_amd64 46.142 MiB
iot_agent_suse_amd64 46.141 MiB

@litianningdatadog litianningdatadog added the aws-microvm Work related to AWS Lambda MicroVM work label Jul 6, 2026
@litianningdatadog
litianningdatadog force-pushed the tianning.li/microvm-07-07-enhanced-usage-metric-instance-tag branch from e08ade8 to 6a1edd1 Compare July 6, 2026 16:34
@litianningdatadog
litianningdatadog force-pushed the tianning.li/microvm-07-06-trace-tag-race-fix branch from 3f06236 to 306b09e Compare July 6, 2026 16:34
@dd-octo-sts

dd-octo-sts Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Files inventory check summary

File checks results against ancestor 1f54e07e:

Results for datadog-agent_7.83.0~devel.git.330.e54ecc4.pipeline.126330100-1_amd64.deb:

No change detected

@cit-pr-commenter-54b7da

cit-pr-commenter-54b7da Bot commented Jul 6, 2026

Copy link
Copy Markdown

Regression Detector

Regression Detector Results

Metrics dashboard
Target profiles
Run ID: 08e2fac1-f929-48d1-acf3-2ba575e40673

Baseline: 1f54e07
Comparison: e54ecc4
Diff

Optimization Goals: ✅ No significant changes detected

Fine details of change detection per experiment

perf experiment goal Δ mean % Δ mean % CI trials links
quality_gate_metrics_logs memory utilization +0.84 [+0.60, +1.09] 1 Logs bounds checks dashboard
quality_gate_security_idle memory utilization +0.20 [+0.14, +0.25] 1 Logs bounds checks dashboard
quality_gate_idle memory utilization +0.15 [+0.10, +0.20] 1 Logs bounds checks dashboard
quality_gate_security_mean_fs_load memory utilization +0.14 [+0.10, +0.17] 1 Logs bounds checks dashboard
quality_gate_security_no_fs_load memory utilization +0.02 [-0.07, +0.11] 1 Logs bounds checks dashboard
quality_gate_private_action_runner memory utilization -0.20 [-0.32, -0.08] 1 Logs bounds checks dashboard
quality_gate_idle_all_features memory utilization -0.69 [-0.73, -0.64] 1 Logs bounds checks dashboard
quality_gate_logs % cpu utilization -1.89 [-2.88, -0.89] 1 Logs bounds checks dashboard

Bounds Checks: ✅ Passed

perf experiment bounds_check_name replicates_passed observed_value links
quality_gate_idle intake_connections 10/10 3 ≤ 4 bounds checks dashboard
quality_gate_idle memory_usage 10/10 148.18MiB ≤ 154MiB bounds checks dashboard
quality_gate_idle total_bytes_received 10/10 738.12KiB ≤ 819.20KiB bounds checks dashboard
quality_gate_idle_all_features intake_connections 10/10 3 ≤ 4 bounds checks dashboard
quality_gate_idle_all_features memory_usage 10/10 494.55MiB ≤ 495MiB bounds checks dashboard
quality_gate_idle_all_features total_bytes_received 10/10 1.13MiB ≤ 1.25MiB bounds checks dashboard
quality_gate_logs intake_connections 10/10 4 ≤ 6 bounds checks dashboard
quality_gate_logs memory_usage 10/10 184.06MiB ≤ 195MiB bounds checks dashboard
quality_gate_logs missed_bytes 10/10 0B = 0B bounds checks dashboard
quality_gate_logs total_bytes_received 10/10 264.16MiB ≤ 292MiB bounds checks dashboard
quality_gate_metrics_logs cpu_usage 10/10 374.85 ≤ 2000 bounds checks dashboard
quality_gate_metrics_logs intake_connections 10/10 3 ≤ 6 bounds checks dashboard
quality_gate_metrics_logs memory_usage 10/10 415.45MiB ≤ 430MiB bounds checks dashboard
quality_gate_metrics_logs missed_bytes 10/10 0B = 0B bounds checks dashboard
quality_gate_metrics_logs total_bytes_received 10/10 0.93GiB ≤ 1.04GiB bounds checks dashboard
quality_gate_private_action_runner memory_usage 10/10 71.30MiB ≤ 75MiB bounds checks dashboard
quality_gate_security_idle cpu_usage 10/10 31.16 ≤ 100 bounds checks dashboard
quality_gate_security_idle memory_usage 10/10 301.84MiB ≤ 330MiB bounds checks dashboard
quality_gate_security_mean_fs_load cpu_usage 10/10 64.07 ≤ 200 bounds checks dashboard
quality_gate_security_mean_fs_load memory_usage 10/10 275.56MiB ≤ 310MiB bounds checks dashboard
quality_gate_security_no_fs_load cpu_usage 10/10 23.08 ≤ 100 bounds checks dashboard
quality_gate_security_no_fs_load memory_usage 10/10 284.92MiB ≤ 320MiB bounds checks dashboard

Explanation

Confidence level: 90.00%
Effect size tolerance: |Δ mean %| ≥ 5.00%

Performance changes are noted in the perf column of each table:

  • ✅ = significantly better comparison variant performance
  • ❌ = significantly worse comparison variant performance
  • ➖ = no significant change in performance

A regression test is an A/B test of target performance in a repeatable rig, where "performance" is measured as "comparison variant minus baseline variant" for an optimization goal (e.g., ingress throughput). Due to intrinsic variability in measuring that goal, we can only estimate its mean value for each experiment; we report uncertainty in that value as a 90.00% confidence interval denoted "Δ mean % CI".

For each experiment, we decide whether a change in performance is a "regression" -- a change worth investigating further -- if all of the following criteria are true:

  1. Its estimated |Δ mean %| ≥ 5.00%, indicating the change is big enough to merit a closer look.

  2. Its 90.00% confidence interval "Δ mean % CI" does not contain zero, indicating that if our statistical model is accurate, there is at least a 90.00% chance there is a difference in performance between baseline and comparison variants.

  3. Its configuration does not mark it "erratic".

Replicate Execution Details

We run multiple replicates for each experiment/variant. However, we allow replicates to be automatically retried if there are any failures, up to 8 times, at which point the replicate is marked dead and we are unable to run analysis for the entire experiment. We call each of these attempts at running replicates a replicate execution. This section lists all replicate executions that failed due to the target crashing or being oom killed.

Note: In the below tables we bucket failures by experiment, variant, and failure type. For each of these buckets we list out the replicate indexes that failed with an annotation signifying how many times said replicate failed with the given failure mode. In the below example the baseline variant of the experiment named experiment_with_failures had two replicates that failed by oom kills. Replicate 0, which failed 8 executions, and replicate 1 which failed 6 executions, all with the same failure mode.

Experiment Variant Replicates Failure Logs Debug Dashboard
experiment_with_failures baseline 0 (x8) 1 (x6) Oom killed Debug Dashboard

The debug dashboard links will take you to a debugging dashboard specifically designed to investigate replicate execution failures.

❌ Retried Profiling Replicate Execution Failures (ddprof)

Note: Profiling replicas may still be executing. See the debug dashboard for up to date status.

Experiment Variant Replicates Failure Debug Dashboard
quality_gate_idle_all_features baseline 10 Oom killed Debug Dashboard
quality_gate_idle_all_features comparison 10 Oom killed Debug Dashboard
quality_gate_metrics_logs baseline 10 Oom killed Debug Dashboard
quality_gate_metrics_logs comparison 10 Oom killed Debug Dashboard
quality_gate_security_idle baseline 10 Crashed (exit code: 134) Debug Dashboard
quality_gate_security_idle comparison 10 Crashed (exit code: 134) Debug Dashboard
quality_gate_security_no_fs_load baseline 10 Crashed (exit code: 134) Debug Dashboard
quality_gate_security_no_fs_load comparison 10 Crashed (exit code: 134) Debug Dashboard

CI Pass/Fail Decision

Passed. All Quality Gates passed.

  • quality_gate_security_idle, bounds check cpu_usage: 10/10 replicas passed. Gate passed.
  • quality_gate_security_idle, bounds check memory_usage: 10/10 replicas passed. Gate passed.
  • quality_gate_private_action_runner, bounds check memory_usage: 10/10 replicas passed. Gate passed.
  • quality_gate_metrics_logs, bounds check cpu_usage: 10/10 replicas passed. Gate passed.
  • quality_gate_metrics_logs, bounds check missed_bytes: 10/10 replicas passed. Gate passed.
  • quality_gate_metrics_logs, bounds check total_bytes_received: 10/10 replicas passed. Gate passed.
  • quality_gate_metrics_logs, bounds check intake_connections: 10/10 replicas passed. Gate passed.
  • quality_gate_metrics_logs, bounds check memory_usage: 10/10 replicas passed. Gate passed.
  • quality_gate_idle, bounds check intake_connections: 10/10 replicas passed. Gate passed.
  • quality_gate_idle, bounds check total_bytes_received: 10/10 replicas passed. Gate passed.
  • quality_gate_idle, bounds check memory_usage: 10/10 replicas passed. Gate passed.
  • quality_gate_logs, bounds check memory_usage: 10/10 replicas passed. Gate passed.
  • quality_gate_logs, bounds check intake_connections: 10/10 replicas passed. Gate passed.
  • quality_gate_logs, bounds check missed_bytes: 10/10 replicas passed. Gate passed.
  • quality_gate_logs, bounds check total_bytes_received: 10/10 replicas passed. Gate passed.
  • quality_gate_security_mean_fs_load, bounds check memory_usage: 10/10 replicas passed. Gate passed.
  • quality_gate_security_mean_fs_load, bounds check cpu_usage: 10/10 replicas passed. Gate passed.
  • quality_gate_idle_all_features, bounds check intake_connections: 10/10 replicas passed. Gate passed.
  • quality_gate_idle_all_features, bounds check total_bytes_received: 10/10 replicas passed. Gate passed.
  • quality_gate_idle_all_features, bounds check memory_usage: 10/10 replicas passed. Gate passed.
  • quality_gate_security_no_fs_load, bounds check memory_usage: 10/10 replicas passed. Gate passed.
  • quality_gate_security_no_fs_load, bounds check cpu_usage: 10/10 replicas passed. Gate passed.

@litianningdatadog
litianningdatadog force-pushed the tianning.li/microvm-07-06-trace-tag-race-fix branch from 306b09e to 20d73ac Compare July 6, 2026 18:54
@litianningdatadog
litianningdatadog requested a review from a team as a code owner July 6, 2026 18:54
@litianningdatadog
litianningdatadog force-pushed the tianning.li/microvm-07-07-enhanced-usage-metric-instance-tag branch from 6a1edd1 to 6e12c30 Compare July 6, 2026 18:56
@litianningdatadog
litianningdatadog requested review from a team and Chronobreak and removed request for a team July 6, 2026 20:23
@litianningdatadog
litianningdatadog force-pushed the tianning.li/microvm-07-06-trace-tag-race-fix branch from 20d73ac to c662ba6 Compare July 7, 2026 00:39
@litianningdatadog
litianningdatadog requested review from a team as code owners July 7, 2026 00:39
@litianningdatadog
litianningdatadog force-pushed the tianning.li/microvm-07-07-enhanced-usage-metric-instance-tag branch from 6e12c30 to c1cf867 Compare July 7, 2026 00:41
@litianningdatadog
litianningdatadog force-pushed the tianning.li/microvm-07-06-trace-tag-race-fix branch from c662ba6 to 55a75b0 Compare July 7, 2026 19:03
@litianningdatadog
litianningdatadog force-pushed the tianning.li/microvm-07-07-enhanced-usage-metric-instance-tag branch from c1cf867 to b6aa664 Compare July 7, 2026 19:04
@litianningdatadog
litianningdatadog force-pushed the tianning.li/microvm-07-06-trace-tag-race-fix branch from 55a75b0 to 8891054 Compare July 7, 2026 21:12
@litianningdatadog
litianningdatadog force-pushed the tianning.li/microvm-07-07-enhanced-usage-metric-instance-tag branch from b6aa664 to 178437f Compare July 7, 2026 21:14
@litianningdatadog
litianningdatadog force-pushed the tianning.li/microvm-07-06-trace-tag-race-fix branch 2 times, most recently from b661d68 to 494d3f6 Compare July 8, 2026 13:29
@litianningdatadog
litianningdatadog force-pushed the tianning.li/microvm-07-07-enhanced-usage-metric-instance-tag branch from 178437f to fbdb435 Compare July 8, 2026 13:30
litianningdatadog added a commit that referenced this pull request Jul 8, 2026
…ister it (#53096)

### What does this PR do?

Completes the MicroVM integration started in #53092/#53093/#53094:

- **`cloudservice/service.go`** — `GetCloudServiceType` now returns
`&MicroVM{}` when `DD_AWS_MICROVM_IMAGE_ARN` is set. This check runs
before the amd64-only arch gate, since MicroVM also supports arm64.
- **`main.go`** — `setup()` builds a `LifecycleContext` from the
telemetry dependencies it already constructs (metric/trace/log flushers,
base log tags, base trace tags) and passes it through
`TracingContext.LifecycleCtx` so `MicroVM.Init` can start the lifecycle
server. This happens on both the normal path and the no-API-key path —
MicroVM's lifecycle handshake with the platform must complete regardless
of whether trace/metric collection is enabled, unlike other cloud
services, which are intentionally not initialized on the no-API-key path
(doing so would create trace spans with tracing disabled and unset span
tags, leading to a nil-map panic on shutdown for e.g. Cloud Run Jobs).
- **`lifecycle/server.go`** — `flushAll` no longer calls
`s.logsFlusher.Flush(...)` unconditionally. The `logsAgent` passed
through as `LifecycleContext.LogsFlusher` can be nil when the logs agent
fails to start (`SetupLogAgent`'s error is discarded in
`cmd/serverless-init/log/log.go`), which would panic during MicroVM's
`/suspend` and `/terminate` handshake. Guarded with a nil check,
matching the existing precedent in `main.go`'s `lastFlush` and
`flushAll`'s own `sampleDrainer` handling.
- **`main_test.go`** — `TestRun_LocalService_SidecarMode` started the
real `RunSidecar` signal-wait path but never sent a signal or cleaned
up, leaking a goroutine that would intercept SIGTERM for the rest of the
test binary's life. It now registers its own SIGTERM listener first (to
override the default terminate disposition before signaling), sends
itself a real SIGTERM, and asserts `RunSidecar` returns.

### Motivation

This is the atomic step that turns on the feature: before this PR,
`MicroVM` (PR 2, fully tested by PR 3/PR 4) exists in the binary but is
unreachable — `GetCloudServiceType` never selects it and `main.go` never
gives it a `LifecycleContext`, so `DD_AWS_MICROVM_IMAGE_ARN` has no
effect. Registration and `main.go` wiring are deliberately kept together
in one PR (rather than splitting further) because registering `MicroVM`
without also wiring `LifecycleContext` would leave `Init` a no-op and
`MicroVM.Run` dereferencing a nil child handle — this PR is the only
point in the stack where that combination is safe to land.

The `flushAll` nil-guard and the sidecar-test signal leak were both
flagged by Codex's automated review of this PR and are fixed here rather
than as follow-ups, since both are one-line-scoped and directly touch
code this PR introduces.

### Update (amended)

This branch was originally built on top of an older, unrestacked copy of
`forwarder.go`/`wire.go`/`server.go` before being reset onto the current
`07-02` base. The reset didn't refresh the working tree, so the original
commit silently reintroduced pre-fix versions of five things (each had a
test pinning it that was deleted alongside it), none of which were
mentioned in the original PR description above:

- **`forwarder.go`** — restores the `CheckRedirect` handler on the
forwarder's `http.Client` so a 3xx from the user app is mirrored to the
platform as-is instead of silently followed (which would replay a POST
hook as a body-dropped GET). See
`TestForwarder_PassThrough_DoesNotFollowRedirects`.
- **`wire.go`** — restores the `sidecarMode` early-return before parsing
`userAppPort` and the forward/ready/validate timeouts, so a stale or
colliding value inherited from an init-mode config produces a warning
instead of failing setup in sidecar mode. See the three
`TestSetupComponents_SidecarMode_*` tests.
- **`lifecycle/server.go`** — restores `writeTimeoutHeadroom`
(`heartbeatStopTimeout + mirrorResponseTimeout`) in the `WriteTimeout`
calculation; restores unconditional response-body buffering in
`handleWithForwarder` for both `flushParallel` (`/suspend`) and
`flushSequential` (`/terminate`) — the `/suspend` path had silently lost
its buffering, risking a partial mirrored body if the parallel flush
outlives `forwardTimeout`; and restores the nil-map guard before writing
`lambda_microvm_id` into a cloned `baseTraceTags` map. See
`TestHandleSuspend_WithForwarder_BodyBufferedBeforeFlush` and
`TestHandleRun_NilBaseTraceTags_DoesNotPanic`.
- **`server_test.go`** — restores `go.uber.org/atomic` in place of
`sync/atomic`, per this repo's `codereview_guideline.md` (this had
already been fixed once, per Codex review on an earlier PR in this
stack, before being reintroduced by the same reset).

Verified via `git merge-tree` that these restorations merge cleanly with
the PRs already stacked on top of this branch (#53104, #53230, #53231).

### Describe how you validated your changes

```
dda inv test --targets=./cmd/serverless-init/...
```

279 tests, 275 passed, 4 skipped (pre-existing platform skips). New
coverage:
- `TestFlushAllNilLogsFlusherDoesNotPanic` — verifies `flushAll`
tolerates a nil `logsFlusher` and still completes promptly (asserts it
returns via the normal "all workers done" path, not the `flushTimeout`
fallback, since a panic in `flushAll`'s spawned goroutine wouldn't be
caught by `assert.NotPanics` alone).
- `TestRun_LocalService_SidecarMode` (revised) — now drives `RunSidecar`
through a real SIGTERM instead of a fixed timeout, and asserts it
returns cleanly with no leaked signal handler.
- `main_test.go` also verifies the `logTagsBase`/`baseTraceTags` values
threaded into `LifecycleContext`, that `metrics.ServerlessMetricAgent`
methods are nil-safe without a started `Demux`, and that
`CloudService.Run` correctly dispatches to both init-container and
sidecar paths.
- `service_test.go` verifies `GetCloudServiceType` selects `MicroVM` and
that it takes priority over `CloudRunJobs` when env vars for both happen
to be set.
- `TestForwarder_PassThrough_DoesNotFollowRedirects`,
`TestSetupComponents_SidecarMode_*` (×3), and
`TestHandleSuspend_WithForwarder_BodyBufferedBeforeFlush`/`TestHandleRun_NilBaseTraceTags_DoesNotPanic`
— restored, per above.

Also ran the linter:

```
dda inv linter.go --targets=./cmd/serverless-init/...
```

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
litianningdatadog added a commit that referenced this pull request Jul 8, 2026
…ister it (#53096)

### What does this PR do?

Completes the MicroVM integration started in #53092/#53093/#53094:

- **`cloudservice/service.go`** — `GetCloudServiceType` now returns
`&MicroVM{}` when `DD_AWS_MICROVM_IMAGE_ARN` is set. This check runs
before the amd64-only arch gate, since MicroVM also supports arm64.
- **`main.go`** — `setup()` builds a `LifecycleContext` from the
telemetry dependencies it already constructs (metric/trace/log flushers,
base log tags, base trace tags) and passes it through
`TracingContext.LifecycleCtx` so `MicroVM.Init` can start the lifecycle
server. This happens on both the normal path and the no-API-key path —
MicroVM's lifecycle handshake with the platform must complete regardless
of whether trace/metric collection is enabled, unlike other cloud
services, which are intentionally not initialized on the no-API-key path
(doing so would create trace spans with tracing disabled and unset span
tags, leading to a nil-map panic on shutdown for e.g. Cloud Run Jobs).
- **`lifecycle/server.go`** — `flushAll` no longer calls
`s.logsFlusher.Flush(...)` unconditionally. The `logsAgent` passed
through as `LifecycleContext.LogsFlusher` can be nil when the logs agent
fails to start (`SetupLogAgent`'s error is discarded in
`cmd/serverless-init/log/log.go`), which would panic during MicroVM's
`/suspend` and `/terminate` handshake. Guarded with a nil check,
matching the existing precedent in `main.go`'s `lastFlush` and
`flushAll`'s own `sampleDrainer` handling.
- **`main_test.go`** — `TestRun_LocalService_SidecarMode` started the
real `RunSidecar` signal-wait path but never sent a signal or cleaned
up, leaking a goroutine that would intercept SIGTERM for the rest of the
test binary's life. It now registers its own SIGTERM listener first (to
override the default terminate disposition before signaling), sends
itself a real SIGTERM, and asserts `RunSidecar` returns.

### Motivation

This is the atomic step that turns on the feature: before this PR,
`MicroVM` (PR 2, fully tested by PR 3/PR 4) exists in the binary but is
unreachable — `GetCloudServiceType` never selects it and `main.go` never
gives it a `LifecycleContext`, so `DD_AWS_MICROVM_IMAGE_ARN` has no
effect. Registration and `main.go` wiring are deliberately kept together
in one PR (rather than splitting further) because registering `MicroVM`
without also wiring `LifecycleContext` would leave `Init` a no-op and
`MicroVM.Run` dereferencing a nil child handle — this PR is the only
point in the stack where that combination is safe to land.

The `flushAll` nil-guard and the sidecar-test signal leak were both
flagged by Codex's automated review of this PR and are fixed here rather
than as follow-ups, since both are one-line-scoped and directly touch
code this PR introduces.

### Update (amended)

This branch was originally built on top of an older, unrestacked copy of
`forwarder.go`/`wire.go`/`server.go` before being reset onto the current
`07-02` base. The reset didn't refresh the working tree, so the original
commit silently reintroduced pre-fix versions of five things (each had a
test pinning it that was deleted alongside it), none of which were
mentioned in the original PR description above:

- **`forwarder.go`** — restores the `CheckRedirect` handler on the
forwarder's `http.Client` so a 3xx from the user app is mirrored to the
platform as-is instead of silently followed (which would replay a POST
hook as a body-dropped GET). See
`TestForwarder_PassThrough_DoesNotFollowRedirects`.
- **`wire.go`** — restores the `sidecarMode` early-return before parsing
`userAppPort` and the forward/ready/validate timeouts, so a stale or
colliding value inherited from an init-mode config produces a warning
instead of failing setup in sidecar mode. See the three
`TestSetupComponents_SidecarMode_*` tests.
- **`lifecycle/server.go`** — restores `writeTimeoutHeadroom`
(`heartbeatStopTimeout + mirrorResponseTimeout`) in the `WriteTimeout`
calculation; restores unconditional response-body buffering in
`handleWithForwarder` for both `flushParallel` (`/suspend`) and
`flushSequential` (`/terminate`) — the `/suspend` path had silently lost
its buffering, risking a partial mirrored body if the parallel flush
outlives `forwardTimeout`; and restores the nil-map guard before writing
`lambda_microvm_id` into a cloned `baseTraceTags` map. See
`TestHandleSuspend_WithForwarder_BodyBufferedBeforeFlush` and
`TestHandleRun_NilBaseTraceTags_DoesNotPanic`.
- **`server_test.go`** — restores `go.uber.org/atomic` in place of
`sync/atomic`, per this repo's `codereview_guideline.md` (this had
already been fixed once, per Codex review on an earlier PR in this
stack, before being reintroduced by the same reset).

Verified via `git merge-tree` that these restorations merge cleanly with
the PRs already stacked on top of this branch (#53104, #53230, #53231).

### Describe how you validated your changes

```
dda inv test --targets=./cmd/serverless-init/...
```

279 tests, 275 passed, 4 skipped (pre-existing platform skips). New
coverage:
- `TestFlushAllNilLogsFlusherDoesNotPanic` — verifies `flushAll`
tolerates a nil `logsFlusher` and still completes promptly (asserts it
returns via the normal "all workers done" path, not the `flushTimeout`
fallback, since a panic in `flushAll`'s spawned goroutine wouldn't be
caught by `assert.NotPanics` alone).
- `TestRun_LocalService_SidecarMode` (revised) — now drives `RunSidecar`
through a real SIGTERM instead of a fixed timeout, and asserts it
returns cleanly with no leaked signal handler.
- `main_test.go` also verifies the `logTagsBase`/`baseTraceTags` values
threaded into `LifecycleContext`, that `metrics.ServerlessMetricAgent`
methods are nil-safe without a started `Demux`, and that
`CloudService.Run` correctly dispatches to both init-container and
sidecar paths.
- `service_test.go` verifies `GetCloudServiceType` selects `MicroVM` and
that it takes priority over `CloudRunJobs` when env vars for both happen
to be set.
- `TestForwarder_PassThrough_DoesNotFollowRedirects`,
`TestSetupComponents_SidecarMode_*` (×3), and
`TestHandleSuspend_WithForwarder_BodyBufferedBeforeFlush`/`TestHandleRun_NilBaseTraceTags_DoesNotPanic`
— restored, per above.

Also ran the linter:

```
dda inv linter.go --targets=./cmd/serverless-init/...
```

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
)

// MicroVM origin product/category/service IDs are a contract with the backend
// intake registry; an accidental change silently mis-attributes metrics.

@Lewis-E Lewis-E Jul 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There's probably a good reason none of the other origin mappings have tests? This is all basically just a restatement of the code. There's no complexity to catch issues in here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The change was introduced in #53030

Comment thread pkg/serverless/trace/span_modifier.go
@litianningdatadog
litianningdatadog force-pushed the tianning.li/microvm-07-06-trace-tag-race-fix branch from a296d8b to 5ab5bf2 Compare July 10, 2026 17:56
@litianningdatadog
litianningdatadog force-pushed the tianning.li/microvm-07-07-enhanced-usage-metric-instance-tag branch from a72bee8 to ac9d1ad Compare July 10, 2026 17:58
@litianningdatadog
litianningdatadog force-pushed the tianning.li/microvm-07-06-trace-tag-race-fix branch from 5ab5bf2 to af3ff1d Compare July 13, 2026 20:41
@litianningdatadog
litianningdatadog force-pushed the tianning.li/microvm-07-07-enhanced-usage-metric-instance-tag branch from ac9d1ad to a48cd46 Compare July 13, 2026 20:42
@github-actions github-actions Bot added the medium review PR review might take time label Jul 15, 2026
@litianningdatadog
litianningdatadog force-pushed the tianning.li/microvm-07-06-trace-tag-race-fix branch from af3ff1d to 3233bdd Compare July 15, 2026 19:23
@litianningdatadog
litianningdatadog force-pushed the tianning.li/microvm-07-07-enhanced-usage-metric-instance-tag branch from a48cd46 to ff03f63 Compare July 15, 2026 19:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes enhanced usage metric cardinality for AWS Lambda MicroVM by attaching a per-instance tag (instance:<id>) at emission time once the /run hook reveals the MicroVM instance ID, avoiding shared mutable state changes to the metric agent.

Changes:

  • Expose lifecycle.Server.InstanceID() as a nil-safe accessor for the captured MicroVM instance ID.
  • Add an optional usageMetricTagsFunc hook to enhancedmetrics.Collector and forward its dynamic tags into AddEnhancedUsageMetric.
  • Wire the hook in main.go via a local usageMetricTagProvider interface, implemented by *cloudservice.MicroVM, plus targeted unit tests.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
cmd/serverless-init/main.go Duck-types cloud service to optionally provide dynamic usage metric tags; passes hook into enhanced metrics collector.
cmd/serverless-init/main_test.go Adds interface-conformance/negative assertions for usageMetricTagProvider wiring.
cmd/serverless-init/lifecycle/server.go Adds exported, nil-safe InstanceID() accessor for instance ID captured from /run.
cmd/serverless-init/lifecycle/server_test.go Adds coverage for InstanceID() behavior before/after /run and on nil receiver.
cmd/serverless-init/enhanced-metrics/collector.go Adds usageMetricTagsFunc to collector and forwards its output as extra tags on each usage metric emission.
cmd/serverless-init/enhanced-metrics/collector_unsupported.go Updates non-Linux stub constructor signature to match the new API.
cmd/serverless-init/enhanced-metrics/collector_test.go Adds tests for dynamic-tag forwarding, nil hook behavior, and constructor wiring.
cmd/serverless-init/cloudservice/microvm.go Implements CurrentUsageMetricTags() to provide the per-instance instance:<id> tag once available.
cmd/serverless-init/cloudservice/microvm_test.go Adds end-to-end tests for CurrentUsageMetricTags() nil/before-run/after-run behavior and interface guard.

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

Comment thread cmd/serverless-init/cloudservice/microvm_test.go Outdated
Comment thread cmd/serverless-init/cloudservice/microvm.go Outdated
@litianningdatadog
litianningdatadog force-pushed the tianning.li/microvm-07-06-trace-tag-race-fix branch from 3233bdd to 965e1f4 Compare July 16, 2026 20:34
@litianningdatadog
litianningdatadog force-pushed the tianning.li/microvm-07-07-enhanced-usage-metric-instance-tag branch 2 times, most recently from dca8f68 to 8a20a70 Compare July 16, 2026 20:52
@litianningdatadog
litianningdatadog force-pushed the tianning.li/microvm-07-06-trace-tag-race-fix branch from 965e1f4 to 25a2304 Compare July 22, 2026 17:42
@litianningdatadog
litianningdatadog force-pushed the tianning.li/microvm-07-07-enhanced-usage-metric-instance-tag branch from 8a20a70 to 99a2b74 Compare July 22, 2026 17:42
…ge metric

Codex flagged on PR #53093 (review comment
#53093 (comment))
that MicroVM's enhanced usage metric (aws.lambda.microvm.instance) is
emitted with the same static tag set for every MicroVM booted from a given
image. MicroVM.GetEnhancedMetricTags returns Usage tags at startup, before
the MicroVM's instance ID is known (it only becomes available once the /run
lifecycle hook fires), and nothing ever adds it afterward. Under normal
autoscaling, with multiple concurrent MicroVMs from the same image, their
usage samples become indistinguishable from each other.

Rather than mutating shared, periodically-read state on
ServerlessMetricAgent (which would need a lock/atomic and broke go vet's
copylocks check, since CloudService.Shutdown takes ServerlessMetricAgent by
value across every cloud service), this attaches the tag at the point of
emission instead:

- lifecycle.Server already tracks the instance ID race-free
  (instanceID *atomic.String, captured in handleRun). Added InstanceID(),
  a nil-safe accessor, so callers outside the lifecycle package can read it.
- MicroVM.CurrentUsageMetricTags() turns that into an "instance:<id>" tag,
  or nil before /run has fired.
- enhanced-metrics.Collector gained an optional usageMetricTagsFunc,
  invoked on every collection tick and passed through the existing
  AddEnhancedUsageMetric(..., extraTags ...string) parameter — no new
  shared mutable state required.
- main.go duck-types cloudService against a local usageMetricTagProvider
  interface to wire this hook; every other cloud service doesn't implement
  it, so usageMetricTagsFunc stays nil and their usage metrics are
  unaffected.

This keeps the change scoped to MicroVM and the (nil-safe, additive)
collector plumbing — no other cloud service, ServerlessMetricAgent, or the
Shutdown interface needed to change.

Unit tests:
  dda inv test --targets=./cmd/serverless-init/...

Amended to address Copilot review feedback on this PR:
- `cloudservice/microvm_test.go`: the new
  TestMicroVM_CurrentUsageMetricTags_AfterRun_ReturnsInstanceTag dialed
  l.Addr().String() directly, which resolves to the unspecified address
  (e.g. [::]:port) since the lifecycle server binds with no host — not a
  guaranteed-valid dial target on every platform. Now dials
  127.0.0.1:<port> explicitly. Also reworded the leftover launchPath/
  "/launch" naming to runPath/"/run" to match the route under test.
- `cloudservice/microvm.go`: CurrentUsageMetricTags' doc comment cited
  exact file:line locations in appservice.go/cloudrun.go, which drift as
  those files change. Now references the method names
  (AppService.GetEnhancedMetricTags, CloudRun.GetEnhancedMetricTags)
  instead.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@litianningdatadog
litianningdatadog force-pushed the tianning.li/microvm-07-06-trace-tag-race-fix branch from 25a2304 to 372f27f Compare July 22, 2026 18:24
@litianningdatadog
litianningdatadog force-pushed the tianning.li/microvm-07-07-enhanced-usage-metric-instance-tag branch from 99a2b74 to e54ecc4 Compare July 22, 2026 18:29
…#53231)

### What does this PR do?

Fixes serverless-init MicroVM lifecycle hook handling against the AWS
Lambda MicroVM hook contract (`lifecycle-model.md` /
`troubleshooting.md`, pinned at commit
`847f477649252b98f8fb828bbfeaf109b57b8cac`).

The main behavior changes are:

1. `/ready` and `/validate` now return fast retryable `503` responses
when the user app is not reachable or forwarding times out, instead of
blocking inside one platform request and sometimes returning `504`.
2. `/validate` now has a 10s default forward budget, so build-time smoke
tests are not constrained by the 1s runtime hook default.
3. The lifecycle HTTP server `WriteTimeout` now includes the fast
reachability dial budget for `/ready` and `/validate`.
4. `/run` now fails with `500` if the platform request body cannot be
read, rather than parsing or forwarding a partial `runHookPayload`.

### Motivation

AWS documents a narrow contract for `/ready` and `/validate`: return
`200` when complete, or return `503` when the application needs more
time so the platform can own the retry loop. Returning another non-200
status, including `504`, can fail image build validation instead of
asking AWS to retry.

The previous forwarder violated that contract in two ways:

- It mapped `context.DeadlineExceeded` to `504` through the shared
runtime-hook error mapper.
- It polled the user app TCP port every 50ms inside one incoming
request, up to the full configured timeout, instead of answering quickly
with `503` and letting the platform retry.

The follow-up fixes cover two edge cases from review: slow-but-valid
`/validate` handlers should get a build-time smoke-test budget, and
`/run` must not act on incomplete platform payloads because the body
carries `microvmId` and any `runHookPayload`.

### Solution

- `PassThroughWaiting` now performs one fast TCP reachability check and
maps unreachable apps or forwarding deadlines to `503`, never `504`.
- `defaultValidateTimeout` is now 30s. Customers can still override it
with `DD_AWS_MICROVM_VALIDATE_TIMEOUT_MS`.
- `NewServer` budgets `dialCheckTimeout + readyTimeout` and
`dialCheckTimeout + validateTimeout` when computing `WriteTimeout`.
- `handleRun` returns `500` on request-body read errors and stops before
setting runtime tags, starting heartbeat, emitting the run metric, or
forwarding the request to the user app.

### Validation

```bash
dda inv test --targets=./cmd/serverless-init/lifecycle/...
dda inv test --targets=./cmd/serverless-init
```

### Additional notes

The lifecycle server still treats no-forwarder MicroVM mode as a
best-effort process-liveness mode. True application-level `/ready` and
`/validate` checks require opting into user-app forwarding with
`DD_AWS_MICROVM_USER_APP_PORT`.

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
@litianningdatadog
litianningdatadog merged commit 4fb5ac6 into tianning.li/microvm-07-06-trace-tag-race-fix Jul 23, 2026
5 of 6 checks passed
@litianningdatadog
litianningdatadog deleted the tianning.li/microvm-07-07-enhanced-usage-metric-instance-tag branch July 23, 2026 17:02
litianningdatadog added a commit that referenced this pull request Jul 23, 2026
…ge metric (#53230)

## Motivation

Codex flagged this on PR #53093 ([review
comment](#53093 (comment))):
MicroVM's enhanced usage metric (`aws.lambda.microvm.instance`) is
emitted with the same static tag set for every MicroVM booted from a
given image.

`MicroVM.GetEnhancedMetricTags` computes the `Usage` tag set once at
startup — before the MicroVM's instance ID is known, since the platform
only reveals it via the `/run` lifecycle hook — and nothing ever updates
it afterward. Under normal autoscaling, with multiple concurrent
MicroVMs running from the same image, their periodic usage samples are
indistinguishable from one another: instance-level usage collapses into
a single series.

## Solution

The obvious fix — mutate the frozen tag set on `ServerlessMetricAgent`
once `/run` reveals the instance ID, mirroring the existing
`LogsTagSetter`/`TraceTagSetter` pattern — turned out to require a
lock/atomic on `ServerlessMetricAgent`. That struct is passed **by
value** into `CloudService.Shutdown` across every cloud service
(`AppService`, `CloudRun`, `CloudRunJobs`, `ContainerApp`,
`LocalService`, `MicroVM`), so adding any lock-bearing field trips `go
vet`'s copylocks check everywhere, not just for MicroVM.

Instead, this attaches the tag at the point of *emission* rather than
mutating shared state:

- `lifecycle.Server` already tracks the instance ID race-free
(`instanceID *atomic.String`, captured in `handleRun`). Added
`InstanceID()`, a nil-safe accessor, so callers outside the `lifecycle`
package can read it.
- `MicroVM.CurrentUsageMetricTags()` turns that into an
`"instance:<id>"` tag, or `nil` before `/run` has fired.
- `enhancedmetrics.Collector` gained an optional `usageMetricTagsFunc`,
invoked on every collection tick and passed straight through the
existing `AddEnhancedUsageMetric(..., extraTags ...string)` parameter —
no new shared mutable state needed.
- `main.go` duck-types `cloudService` against a local
`usageMetricTagProvider` interface to wire this hook. Every other cloud
service doesn't implement it, so `usageMetricTagsFunc` stays `nil` for
them and their usage metrics are unaffected.

This keeps the change scoped to MicroVM plus small, nil-safe, additive
plumbing in the shared collector — no other cloud service,
`ServerlessMetricAgent`, or the `Shutdown` interface needed to change.

## Test plan

- [x] `dda inv test --targets=./cmd/serverless-init/...` — all 281 tests
pass (12 new: `InstanceID()` nil/before/after `/run`,
`CurrentUsageMetricTags()` nil-server/before-run/after-run end-to-end,
collector dynamic-tag forwarding + nil-func regression guard,
`NewCollector` wiring, and the `usageMetricTagProvider` type-assertion
pinning `*MicroVM` in vs. every other cloud service out)
- [x] `gofmt -l` clean on all changed files
- [x] `dda inv linter.go --targets=./cmd/serverless-init/...` — 0 issues
- [ ]
`cmd/serverless-init/enhanced-metrics/collector.go`/`collector_test.go`
carry a pre-existing `//go:build linux` tag; this session ran on macOS
without a Linux cross-toolchain, so those specific new tests are
syntax-checked (`gofmt -e`) and reviewed but not yet executed — first
real run will be in Linux CI

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
litianningdatadog added a commit that referenced this pull request Jul 27, 2026
…ge metric (#53230)

Codex flagged this on PR #53093 ([review
comment](#53093 (comment))):
MicroVM's enhanced usage metric (`aws.lambda.microvm.instance`) is
emitted with the same static tag set for every MicroVM booted from a
given image.

`MicroVM.GetEnhancedMetricTags` computes the `Usage` tag set once at
startup — before the MicroVM's instance ID is known, since the platform
only reveals it via the `/run` lifecycle hook — and nothing ever updates
it afterward. Under normal autoscaling, with multiple concurrent
MicroVMs running from the same image, their periodic usage samples are
indistinguishable from one another: instance-level usage collapses into
a single series.

The obvious fix — mutate the frozen tag set on `ServerlessMetricAgent`
once `/run` reveals the instance ID, mirroring the existing
`LogsTagSetter`/`TraceTagSetter` pattern — turned out to require a
lock/atomic on `ServerlessMetricAgent`. That struct is passed **by
value** into `CloudService.Shutdown` across every cloud service
(`AppService`, `CloudRun`, `CloudRunJobs`, `ContainerApp`,
`LocalService`, `MicroVM`), so adding any lock-bearing field trips `go
vet`'s copylocks check everywhere, not just for MicroVM.

Instead, this attaches the tag at the point of *emission* rather than
mutating shared state:

- `lifecycle.Server` already tracks the instance ID race-free
(`instanceID *atomic.String`, captured in `handleRun`). Added
`InstanceID()`, a nil-safe accessor, so callers outside the `lifecycle`
package can read it.
- `MicroVM.CurrentUsageMetricTags()` turns that into an
`"instance:<id>"` tag, or `nil` before `/run` has fired.
- `enhancedmetrics.Collector` gained an optional `usageMetricTagsFunc`,
invoked on every collection tick and passed straight through the
existing `AddEnhancedUsageMetric(..., extraTags ...string)` parameter —
no new shared mutable state needed.
- `main.go` duck-types `cloudService` against a local
`usageMetricTagProvider` interface to wire this hook. Every other cloud
service doesn't implement it, so `usageMetricTagsFunc` stays `nil` for
them and their usage metrics are unaffected.

This keeps the change scoped to MicroVM plus small, nil-safe, additive
plumbing in the shared collector — no other cloud service,
`ServerlessMetricAgent`, or the `Shutdown` interface needed to change.

- [x] `dda inv test --targets=./cmd/serverless-init/...` — all 281 tests
pass (12 new: `InstanceID()` nil/before/after `/run`,
`CurrentUsageMetricTags()` nil-server/before-run/after-run end-to-end,
collector dynamic-tag forwarding + nil-func regression guard,
`NewCollector` wiring, and the `usageMetricTagProvider` type-assertion
pinning `*MicroVM` in vs. every other cloud service out)
- [x] `gofmt -l` clean on all changed files
- [x] `dda inv linter.go --targets=./cmd/serverless-init/...` — 0 issues
- [ ]
`cmd/serverless-init/enhanced-metrics/collector.go`/`collector_test.go`
carry a pre-existing `//go:build linux` tag; this session ran on macOS
without a Linux cross-toolchain, so those specific new tests are
syntax-checked (`gofmt -e`) and reviewed but not yet executed — first
real run will be in Linux CI

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
litianningdatadog added a commit that referenced this pull request Aug 13, 2026
…ge metric (#53230)

Codex flagged this on PR #53093 ([review
comment](#53093 (comment))):
MicroVM's enhanced usage metric (`aws.lambda.microvm.instance`) is
emitted with the same static tag set for every MicroVM booted from a
given image.

`MicroVM.GetEnhancedMetricTags` computes the `Usage` tag set once at
startup — before the MicroVM's instance ID is known, since the platform
only reveals it via the `/run` lifecycle hook — and nothing ever updates
it afterward. Under normal autoscaling, with multiple concurrent
MicroVMs running from the same image, their periodic usage samples are
indistinguishable from one another: instance-level usage collapses into
a single series.

The obvious fix — mutate the frozen tag set on `ServerlessMetricAgent`
once `/run` reveals the instance ID, mirroring the existing
`LogsTagSetter`/`TraceTagSetter` pattern — turned out to require a
lock/atomic on `ServerlessMetricAgent`. That struct is passed **by
value** into `CloudService.Shutdown` across every cloud service
(`AppService`, `CloudRun`, `CloudRunJobs`, `ContainerApp`,
`LocalService`, `MicroVM`), so adding any lock-bearing field trips `go
vet`'s copylocks check everywhere, not just for MicroVM.

Instead, this attaches the tag at the point of *emission* rather than
mutating shared state:

- `lifecycle.Server` already tracks the instance ID race-free
(`instanceID *atomic.String`, captured in `handleRun`). Added
`InstanceID()`, a nil-safe accessor, so callers outside the `lifecycle`
package can read it.
- `MicroVM.CurrentUsageMetricTags()` turns that into an
`"instance:<id>"` tag, or `nil` before `/run` has fired.
- `enhancedmetrics.Collector` gained an optional `usageMetricTagsFunc`,
invoked on every collection tick and passed straight through the
existing `AddEnhancedUsageMetric(..., extraTags ...string)` parameter —
no new shared mutable state needed.
- `main.go` duck-types `cloudService` against a local
`usageMetricTagProvider` interface to wire this hook. Every other cloud
service doesn't implement it, so `usageMetricTagsFunc` stays `nil` for
them and their usage metrics are unaffected.

This keeps the change scoped to MicroVM plus small, nil-safe, additive
plumbing in the shared collector — no other cloud service,
`ServerlessMetricAgent`, or the `Shutdown` interface needed to change.

- [x] `dda inv test --targets=./cmd/serverless-init/...` — all 281 tests
pass (12 new: `InstanceID()` nil/before/after `/run`,
`CurrentUsageMetricTags()` nil-server/before-run/after-run end-to-end,
collector dynamic-tag forwarding + nil-func regression guard,
`NewCollector` wiring, and the `usageMetricTagProvider` type-assertion
pinning `*MicroVM` in vs. every other cloud service out)
- [x] `gofmt -l` clean on all changed files
- [x] `dda inv linter.go --targets=./cmd/serverless-init/...` — 0 issues
- [ ]
`cmd/serverless-init/enhanced-metrics/collector.go`/`collector_test.go`
carry a pre-existing `//go:build linux` tag; this session ran on macOS
without a Linux cross-toolchain, so those specific new tests are
syntax-checked (`gofmt -e`) and reviewed but not yet executed — first
real run will be in Linux CI

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
litianningdatadog added a commit that referenced this pull request Aug 13, 2026
…ge metric (#53230)

Codex flagged this on PR #53093 ([review
comment](#53093 (comment))):
MicroVM's enhanced usage metric (`aws.lambda.microvm.instance`) is
emitted with the same static tag set for every MicroVM booted from a
given image.

`MicroVM.GetEnhancedMetricTags` computes the `Usage` tag set once at
startup — before the MicroVM's instance ID is known, since the platform
only reveals it via the `/run` lifecycle hook — and nothing ever updates
it afterward. Under normal autoscaling, with multiple concurrent
MicroVMs running from the same image, their periodic usage samples are
indistinguishable from one another: instance-level usage collapses into
a single series.

The obvious fix — mutate the frozen tag set on `ServerlessMetricAgent`
once `/run` reveals the instance ID, mirroring the existing
`LogsTagSetter`/`TraceTagSetter` pattern — turned out to require a
lock/atomic on `ServerlessMetricAgent`. That struct is passed **by
value** into `CloudService.Shutdown` across every cloud service
(`AppService`, `CloudRun`, `CloudRunJobs`, `ContainerApp`,
`LocalService`, `MicroVM`), so adding any lock-bearing field trips `go
vet`'s copylocks check everywhere, not just for MicroVM.

Instead, this attaches the tag at the point of *emission* rather than
mutating shared state:

- `lifecycle.Server` already tracks the instance ID race-free
(`instanceID *atomic.String`, captured in `handleRun`). Added
`InstanceID()`, a nil-safe accessor, so callers outside the `lifecycle`
package can read it.
- `MicroVM.CurrentUsageMetricTags()` turns that into an
`"instance:<id>"` tag, or `nil` before `/run` has fired.
- `enhancedmetrics.Collector` gained an optional `usageMetricTagsFunc`,
invoked on every collection tick and passed straight through the
existing `AddEnhancedUsageMetric(..., extraTags ...string)` parameter —
no new shared mutable state needed.
- `main.go` duck-types `cloudService` against a local
`usageMetricTagProvider` interface to wire this hook. Every other cloud
service doesn't implement it, so `usageMetricTagsFunc` stays `nil` for
them and their usage metrics are unaffected.

This keeps the change scoped to MicroVM plus small, nil-safe, additive
plumbing in the shared collector — no other cloud service,
`ServerlessMetricAgent`, or the `Shutdown` interface needed to change.

- [x] `dda inv test --targets=./cmd/serverless-init/...` — all 281 tests
pass (12 new: `InstanceID()` nil/before/after `/run`,
`CurrentUsageMetricTags()` nil-server/before-run/after-run end-to-end,
collector dynamic-tag forwarding + nil-func regression guard,
`NewCollector` wiring, and the `usageMetricTagProvider` type-assertion
pinning `*MicroVM` in vs. every other cloud service out)
- [x] `gofmt -l` clean on all changed files
- [x] `dda inv linter.go --targets=./cmd/serverless-init/...` — 0 issues
- [ ]
`cmd/serverless-init/enhanced-metrics/collector.go`/`collector_test.go`
carry a pre-existing `//go:build linux` tag; this session ran on macOS
without a Linux cross-toolchain, so those specific new tests are
syntax-checked (`gofmt -e`) and reviewed but not yet executed — first
real run will be in Linux CI

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
litianningdatadog added a commit that referenced this pull request Aug 14, 2026
…ge metric (#53230)

Codex flagged this on PR #53093 ([review
comment](#53093 (comment))):
MicroVM's enhanced usage metric (`aws.lambda.microvm.instance`) is
emitted with the same static tag set for every MicroVM booted from a
given image.

`MicroVM.GetEnhancedMetricTags` computes the `Usage` tag set once at
startup — before the MicroVM's instance ID is known, since the platform
only reveals it via the `/run` lifecycle hook — and nothing ever updates
it afterward. Under normal autoscaling, with multiple concurrent
MicroVMs running from the same image, their periodic usage samples are
indistinguishable from one another: instance-level usage collapses into
a single series.

The obvious fix — mutate the frozen tag set on `ServerlessMetricAgent`
once `/run` reveals the instance ID, mirroring the existing
`LogsTagSetter`/`TraceTagSetter` pattern — turned out to require a
lock/atomic on `ServerlessMetricAgent`. That struct is passed **by
value** into `CloudService.Shutdown` across every cloud service
(`AppService`, `CloudRun`, `CloudRunJobs`, `ContainerApp`,
`LocalService`, `MicroVM`), so adding any lock-bearing field trips `go
vet`'s copylocks check everywhere, not just for MicroVM.

Instead, this attaches the tag at the point of *emission* rather than
mutating shared state:

- `lifecycle.Server` already tracks the instance ID race-free
(`instanceID *atomic.String`, captured in `handleRun`). Added
`InstanceID()`, a nil-safe accessor, so callers outside the `lifecycle`
package can read it.
- `MicroVM.CurrentUsageMetricTags()` turns that into an
`"instance:<id>"` tag, or `nil` before `/run` has fired.
- `enhancedmetrics.Collector` gained an optional `usageMetricTagsFunc`,
invoked on every collection tick and passed straight through the
existing `AddEnhancedUsageMetric(..., extraTags ...string)` parameter —
no new shared mutable state needed.
- `main.go` duck-types `cloudService` against a local
`usageMetricTagProvider` interface to wire this hook. Every other cloud
service doesn't implement it, so `usageMetricTagsFunc` stays `nil` for
them and their usage metrics are unaffected.

This keeps the change scoped to MicroVM plus small, nil-safe, additive
plumbing in the shared collector — no other cloud service,
`ServerlessMetricAgent`, or the `Shutdown` interface needed to change.

- [x] `dda inv test --targets=./cmd/serverless-init/...` — all 281 tests
pass (12 new: `InstanceID()` nil/before/after `/run`,
`CurrentUsageMetricTags()` nil-server/before-run/after-run end-to-end,
collector dynamic-tag forwarding + nil-func regression guard,
`NewCollector` wiring, and the `usageMetricTagProvider` type-assertion
pinning `*MicroVM` in vs. every other cloud service out)
- [x] `gofmt -l` clean on all changed files
- [x] `dda inv linter.go --targets=./cmd/serverless-init/...` — 0 issues
- [ ]
`cmd/serverless-init/enhanced-metrics/collector.go`/`collector_test.go`
carry a pre-existing `//go:build linux` tag; this session ran on macOS
without a Linux cross-toolchain, so those specific new tests are
syntax-checked (`gofmt -e`) and reviewed but not yet executed — first
real run will be in Linux CI

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aws-microvm Work related to AWS Lambda MicroVM work medium review PR review might take time

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants