Merge remote-tracking branch 'origin/main' into taegyunkim/prof-14213-timer-create

Author: taegyunkim

.claude/skills/dependency-direction-analysis/SKILL.md

@@ -0,0 +1,278 @@
+---
+
+name: dependency-direction-analysis
+description: >
+  Run the dependency direction detector against ddtrace and propose architectural
+  fixes for any violations found. Use this when adding or refactoring modules
+  under ddtrace/internal, ddtrace/contrib, or any product package, or when the
+  detect_layering_violations CI job reports new violations on a PR.
+allowed-tools:
+  - Bash
+  - Read
+  - Grep
+  - Glob
+  - Edit
+  - TodoWrite
+---
+
+# Dependency Direction Analysis Skill
+
+This skill runs the dependency direction detector locally and proposes sound
+architectural fixes for any violations found. It enforces two rules:
+
+1. **`ddtrace.internal` and `ddtrace.contrib` must not depend on product code.**
+   They are shared foundation layers; every product depends on them, so a
+   dependency running the other way creates hidden coupling and risks circular
+   imports (see the `circular-import-analysis` skill).
+2. **Products must not depend on each other directly.** Tracing, AppSec, AI
+   Guard, LLM Observability, Profiling, Dynamic Instrumentation, CI
+   Visibility, Error Tracking, OpenFeature, OpenTelemetry, and Runtime metrics
+   are each isolated: none of them are mandatory for a given dd-trace-py
+   install, so one product can't assume another is present.
+
+The guiding principle is the same as circular-import analysis: **Separation of
+Concerns**. Fixes must restructure ownership or add a decoupling layer, not
+paper over the problem with deferred imports.
+
+## When to Use This Skill
+
+- The `detect_layering_violations` CI job reports new violations on your PR.
+- You are adding a new module, or an import, that crosses from `ddtrace/internal`,
+  `ddtrace/contrib`, or one product package into another product package.
+- You are adding a brand new top-level `ddtrace/<x>` package or module and the
+  CI job reports it as an uncovered/uncategorized top-level module.
+- You are refactoring and want to verify you haven't introduced a new violation.
+
+## Running the Analysis
+
+```bash
+uv run --script scripts/import-analysis/layers.py analyze violations.json
+```
+
+This writes the results to `violations.json` and prints a summary to stdout.
+Requires `uv` on `PATH` (`brew install uv` or `pip install uv`). The output has
+two top-level keys:
+
+```json
+{
+  "violations": [ ... ],
+  "uncovered": [ "ddtrace.newthing" ]
+}
+```
+
+`violations` entries look like:
+```json
+{
+  "from": "ddtrace.internal.tracemethods",
+  "to": "ddtrace.trace",
+  "from_zone": "internal-core",
+  "to_zone": "product:tracing",
+  "score": 139,
+  "in_tangle": true
+}
+```
+
+- `from` / `to` — the two modules the violating import connects.
+- `from_zone` / `to_zone` — which side of the rule they fall on (`internal-core`,
+  `contrib`, or `product:<name>`).
+- `score` — how bad this specific edge is (see "Severity scoring" below).
+- `in_tangle` — the imported module is also part of a strongly connected
+  component larger than one module, i.e. this violation is compounding an
+  existing circular-import problem, not just crossing a boundary once.
+
+`uncovered` lists direct children of the `ddtrace` package root (packages or
+`.py` modules) that are neither a key in `layers.json`'s `zones` map nor listed
+in `foundation.top_level`. This is what catches a new top-level submodule that
+was added without anyone deciding which zone it belongs to — without it, a new
+package like `ddtrace/newproduct/` would silently be treated as exempt
+foundation code and get zero dependency-direction enforcement. Unlike
+violations, a new entry here always fails CI on `compare` (see below),
+regardless of severity — it represents a config gap, not a graded issue.
+
+To compare against the base branch the way CI does (new vs. pre-existing vs.
+worsened vs. removed, for both violations and uncovered modules):
+
+```bash
+uv run --script scripts/import-analysis/layers.py compare violations-base.json violations-pr.json
+```
+
+Clean up afterwards:
+```bash
+rm violations.json violations-base.json violations-pr.json
+```
+
+## Zone Configuration
+
+Zones are defined in `scripts/import-analysis/layers.json`, keyed by module
+prefix (longest match wins), so a product's own `ddtrace.internal.<product>`
+subpackage (e.g. `ddtrace.internal.appsec`) is carved out of the
+`ddtrace.internal` catch-all and treated as part of that product, not as
+foundation code. Modules with no matching prefix (e.g. `ddtrace.ext`,
+`ddtrace.propagation`, `ddtrace.vendor`) are unclassified "foundation" code and
+are exempt from every rule, both as importer and as imported module.
+
+`layers.json` also has an `exceptions` list of zone-pairs that are deliberately
+exempt from the rules — this is how we record a considered decision without
+touching detection logic. For example, `ddtrace/contrib/*` modules are tracer
+integrations by design, so `contrib -> product:tracing` is listed as an
+exception rather than flagged on every run.
+
+**Only add an exception when the dependency is intentional and durable** — not
+as a shortcut to make CI pass. If you're unsure whether an edge should be an
+exception or a bug, ask; this is a business/architecture decision, not
+something to infer from the code.
+
+### Fixing a new "uncovered top-level module" finding
+
+When the CI job (or `analyze`) reports a new entry under `uncovered`, someone
+added a new direct child of `ddtrace/` (a package or a `.py` module) that
+`layers.json` doesn't know about yet. Resolve it by editing
+`scripts/import-analysis/layers.json`:
+
+- If it's a new product (mandatory-or-not feature area, isolated from other
+  products), add it to `zones` as `"ddtrace.<name>": "product:<name>"`, and
+  add its `ddtrace.internal.<name>` counterpart too if one exists.
+- If it's shared foundation code that everything may depend on and that
+  itself has no restrictions (like `ddtrace.ext` or `ddtrace.propagation`),
+  add it to `foundation.top_level`.
+- If it's a carve-out of an existing product (e.g. a new
+  `ddtrace.internal.<product>` subpackage), map it to that product's zone
+  rather than leaving it to fall through to `internal-core`.
+
+Don't add it to `foundation.top_level` just to silence the check — that
+defeats the point of the coverage check. Ask if it's unclear which zone fits.
+
+## Severity Scoring
+
+Each violation's `score` combines three structural signals (no git history
+involved):
+
+- **Rule weight** — `internal-core`/`contrib` violations start higher (3) than
+  product-vs-product violations (1), because foundation code reaching upward
+  is a worse inversion than two peers leaking into each other.
+- **Afferent coupling of the target** (`ca` from betsy's `ModuleMetrics`) — how
+  many other modules already depend on the module being imported. A violation
+  that reaches into a heavily-relied-upon module has a bigger blast radius to
+  eventually unwind.
+- **Cycle bonus (+5)** — added when the imported module's `nccd` (from betsy)
+  is greater than 1.0, i.e. it's already part of an import tangle. Fixing the
+  layering violation first often makes the tangle easier to break too.
+
+Use the score to prioritize: fix the highest-scoring violations first,
+especially any marked `in_tangle`.
+
+## Architectural Patterns for Fixing Violations
+
+> **Never use deferred imports (`import x` inside a function body) as a fix.**
+> They hide the structural problem and impose a runtime cost on every call.
+
+### Understand the edge first
+
+```bash
+# What exactly does <from> import from <to>?
+grep -n "^import ddtrace\|^from ddtrace" <path/to/from/module>.py
+```
+
+Identify the exact names crossing the boundary before choosing a fix — often
+only a small fraction of the target module is actually needed.
+
+---
+
+### Pattern 1 — Core event bus (for `contrib` -> product violations)
+
+**When to use:** A contrib integration wants to notify or be observed by a
+product (this is the most common shape for `contrib -> product:X`
+violations). This is the documented pattern in
+`.cursor/rules/isolated-responsibility.mdc`.
+
+The contrib patch dispatches an event; it does not import the product:
+
+```python
+from ddtrace.internal import core
+
+core.dispatch(f"{event}.before", (kwargs,), allow_raise=True)
+resp = func(*args, **kwargs)
+core.dispatch(f"{event}.after", (kwargs, resp), allow_raise=True)
+```
+
+The product registers a listener, guarded by its own enable flag, inside its
+own package — not inside `contrib`:
+
+```python
+from ddtrace.internal import core
+
+def load_my_product():
+    core.on("some.integration.before", _before_handler)
+```
+
+Neither side imports the other; `ddtrace.internal.core` is foundation code
+both may depend on.
+
+---
+
+### Pattern 2 — Dependency inversion (for `internal-core` -> product violations)
+
+**When to use:** `ddtrace.internal` needs to call into a product, but the
+product also needs to be the one driving behavior (e.g. registering a hook,
+supplying a callback).
+
+Define a `Protocol` or abstract base inside `ddtrace.internal` (or a small
+neutral module); the product implements it and registers itself explicitly.
+`ddtrace.internal` depends on the abstraction, never on the concrete product
+package.
+
+---
+
+### Pattern 3 — Extract shared types into a third, unclassified module
+
+**When to use:** Two zones share a data type, constant, or protocol that both
+legitimately need, but neither should own.
+
+Create a thin module outside both zones' prefixes (so it's unclassified
+foundation code, e.g. `ddtrace._types` or similar) containing only the shared
+contract. Both sides import from it; neither imports from the other.
+
+---
+
+### Pattern 4 — Move the code to the zone that owns it
+
+**When to use:** The violation exists because a function/class ended up in
+the wrong package. This is the simplest and often best fix.
+
+If `ddtrace.internal.tracemethods` calls something that conceptually belongs
+to the tracing product, move it into `ddtrace.trace`/`ddtrace._trace` so the
+dependency direction reverses: the product depends on internal-core (allowed),
+not the other way round.
+
+---
+
+### Pattern 5 — Question whether the target should be foundation code
+
+**When to use:** A product-to-product violation involves a genuinely
+general-purpose utility that happens to live inside a product package (e.g.
+a formatting helper under `ddtrace.trace` that other products also want).
+
+Move the utility down into `ddtrace.internal` (or an unclassified module) so
+every product can depend on it without depending on each other. Don't do this
+for anything that's conceptually part of the product's public contract (e.g.
+`Tracer`, `Span`) — those stay put, and the dependency on them should go
+through Pattern 1 or 2 instead.
+
+---
+
+## Decision checklist before proposing a fix
+
+1. **Identify the exact cross-boundary names** — grep the violating file.
+2. **Classify the relationship:**
+   - Contrib notifying/observing a product → Pattern 1 (core event bus)
+   - internal-core needs product behavior → Pattern 2 (dependency inversion)
+   - Shared data type/constant → Pattern 3 (extract)
+   - Wrong home for the code → Pattern 4 (move)
+   - Misplaced general-purpose utility → Pattern 5 (relocate to foundation)
+3. **Consider whether this is actually an intentional, durable dependency** —
+   if so, propose adding it to `layers.json`'s `exceptions` list instead of
+   restructuring code, but say so explicitly and explain why; this is a call
+   for the humans reviewing the PR, not something to decide unilaterally.
+4. **Verify** by re-running `uv run --script scripts/import-analysis/layers.py analyze violations.json`
+   after the change and confirming the violation is gone (or, if compared
+   against a saved base snapshot, that it doesn't appear as new).

.github/actions/generated-change-patch/action.yml

@@ -0,0 +1,116 @@
+name: Generated change patch
+description: Create or safely apply a patch containing generated version updates
+
+inputs:
+  mode:
+    description: Whether to create or apply the patch
+    required: true
+  patch-file:
+    description: Path to the patch artifact
+    required: true
+  profile:
+    description: Allowlist of generated paths to enforce
+    required: true
+
+outputs:
+  changed:
+    description: Whether the generated patch contains changes
+    value: ${{ steps.patch.outputs.changed }}
+
+runs:
+  using: composite
+  steps:
+    # AIDEV-NOTE: Keep patch application and validation in this single shell step,
+    # and do not add repository-mutating steps between this action and PR creation.
+    # The patch is untrusted data and must not replace code that is executed later.
+    - id: patch
+      shell: bash
+      env:
+        MODE: ${{ inputs.mode }}
+        PATCH_FILE: ${{ inputs.patch-file }}
+        PROFILE: ${{ inputs.profile }}
+      run: |
+        set -euo pipefail
+
+        validate_path() {
+          local path=$1
+
+          case "$PROFILE" in
+            package-versions)
+              if [[ "$path" =~ ^\.riot/requirements/[^/]+\.txt$ ]] ||
+                [[ "$path" == "supported_versions.json" ]] ||
+                [[ "$path" == "scripts/integration_registry/registry.yaml" ]]; then
+                return
+              fi
+              ;;
+            supported-versions)
+              if [[ "$path" == "supported_versions.json" ]]; then
+                return
+              fi
+              ;;
+            *)
+              printf '::error::Unknown generated-change profile: %q\n' "$PROFILE"
+              exit 1
+              ;;
+          esac
+
+          printf '::error::Unexpected generated change: %q\n' "$path"
+          exit 1
+        }
+
+        validate_mode() {
+          local path=$1
+          local mode
+          mode=$(git ls-files -s -- "$path" | awk '{print $1}')
+          if [[ -n "$mode" && "$mode" != "100644" ]]; then
+            printf '::error::Unexpected file mode %q for %q\n' "$mode" "$path"
+            exit 1
+          fi
+        }
+
+        if [[ "$MODE" == "create" ]]; then
+          while IFS= read -r -d '' path; do
+            validate_path "$path"
+            git add --intent-to-add -- "$path"
+          done < <(git ls-files --others --exclude-standard -z)
+
+          if git diff --quiet HEAD --; then
+            echo "changed=false" >> "$GITHUB_OUTPUT"
+            exit 0
+          fi
+
+          while IFS= read -r -d '' path; do
+            validate_path "$path"
+            validate_mode "$path"
+          done < <(git diff --name-only -z HEAD --)
+
+          # Keep generated artifacts clean before they cross into the PR-creation job.
+          git diff --check HEAD --
+          git diff --binary --full-index --no-ext-diff HEAD -- > "$PATCH_FILE"
+          echo "changed=true" >> "$GITHUB_OUTPUT"
+          exit 0
+        fi
+
+        if [[ "$MODE" == "apply" ]]; then
+          git apply --check --index "$PATCH_FILE"
+          git apply --index "$PATCH_FILE"
+
+          changed=false
+          while IFS= read -r -d '' path; do
+            changed=true
+            validate_path "$path"
+            validate_mode "$path"
+          done < <(git diff --cached --name-only -z)
+
+          if [[ "$changed" != "true" ]]; then
+            echo "::error::Generated patch contains no changes"
+            exit 1
+          fi
+
+          git diff --cached --check
+          echo "changed=true" >> "$GITHUB_OUTPUT"
+          exit 0
+        fi
+
+        printf '::error::Unknown generated-change mode: %q\n' "$MODE"
+        exit 1