Skip to content

Fix generated-class source attribution in MAL, LAL and OAL - #13971

Open
wu-sheng wants to merge 1 commit into
masterfrom
feat/mal-line-attribution
Open

Fix generated-class source attribution in MAL, LAL and OAL#13971
wu-sheng wants to merge 1 commit into
masterfrom
feat/mal-line-attribution

Conversation

@wu-sheng

@wu-sheng wu-sheng commented Aug 8, 2026

Copy link
Copy Markdown
Member

Fix generated-class source attribution in MAL, LAL and OAL

When generated DSL code throws, the stack frame should name the file it came from, the line inside that file, and the line of the rule that produced it. All three were broken.

at ...rt.envoy_ca_L41_service_listener_ssl_ca_expiration_seconds.run(
       envoy_ca_L41_service_listener_ssl_ca_expiration_seconds.java:12)
                └─┬─┘                                          └───┬───┘  └┬┘
      envoy-ca.yaml line 41                              a real file    line 12
broken fix
_L<n> was the rules-list index, so every file's first rule read L0 the rule's real YAML line
SourceFile named a file that was never written names the sidecar actually on disk
LineNumberTable held statement ordinals MAL maps each statement to its real sidecar line

Fixing any two of the three leaves the frame useless, which is why the class-naming issue is part of this rather than cosmetic.

Closure companions

A companion always claimed a source file — Javassist stamps SourceFile unconditionally — but the file was never written: 84 dangling references in network-profiling alone. Each now writes its own sidecar and carries one entry at its SAM signature. One entry is derived, not conceded: the statement scanner marks boundaries at stores to a result slot, which a closure body never makes. On the shipped rule set 96% of companions would collapse to one entry regardless, while the forEach ones over-count — 8 boundaries for 5 statements.

0/24 → 24/24 companions with a resolvable sidecar and a line table.

LAL and OAL

Same defect: both stamped (execution-basic.yaml:304)auto-layer-not-set.java while writing execution_basic_L304_auto_layer_not_set.java. The provenance format is what made it convincing — it reads as more informative than a bare file name, which is why nobody noticed it addressed nothing. LAL: 0/48 → 48/48 resolvable.

Their line tables are removed rather than corrected. Their statements are void invocations that store nothing, so MAL's boundary detector finds nothing, and the stack-depth alternative is unsound (the probe's ifeq consumes its operand mid-statement). While SourceFile dangled, an ordinal was inert; once it resolves, an ordinal resolves too — to the sidecar's comment header. A single signature-line entry for them is a follow-up.

Both sidecar writers also moved off a platform-default FileWriter with a non-ASCII header — the pairing that produced MalformedInputException for MAL on this branch.

  • Add a unit test to verify that the fix works.
  • Explain briefly why the bug exists and how to fix it.

Scope

The error path only. No API, no payload, no format change. The dsl-debugging probes, records and UI are untouched; their line numbers come from parse-time ANTLR/SnakeYAML tokens baked in as int literals, which never involved bytecode. Verified: getStackTrace|StackTraceElement has 0 hits across oap-server, apm-protocol, apm-webapp — no production code reads a stack frame, which is also why this survived so long.

Known gaps

  • Hierarchy and OAL's metrics-builder write no sidecar, so their SourceFile still addresses nothing. Adding a writer is a feature, not a fix; recorded rather than half-fixed.
  • LAL and OAL frames resolve to a file but carry no line, pending the single-entry follow-up.
  • Zabbix loads its YAML without stamping, so its rules get _Lunknown_ names. The variant it needs (MalYamlLineIndex.index(yaml, "metrics")) exists and is unit-tested; only the loader wiring is missing.

Testing

The stack frame's three coordinates each have a dedicated test: classNameCarriesTheRealYamlLine (the _L37_ segment on a real compiled rule), sourceFileNamesTheFileThatWasActuallyWritten, and everyLineNumberEntryPointsAtAStatementInThatFile / theLastBoundaryIsTheReturn.

Line-number fixtures are test-only DSL, in both MAL and LAL. Shipped rules were read to learn the shape worth mimicking — a tag({...}) closure in expPrefix/expSuffix is the majority pattern — but are never used as fixtures, so a rule edit cannot break a line assertion.

Every behaviour fails without its fix:

mutation failure
drop the companion sidecar no companion sidecar carries the expPrefix closure body
COMPANION_SAM_LINE_IN_CLASS 9 → 10 line 10 should hold the SAM signature but was: 'java.util.Map tags = ...'
restore the LAL provenance format expected: <..._2.java> but was: <(execution-basic.yaml:304)..._2.java>
restore LAL's ordinal table found 4 entries, first pointing at line 1
drop > 0 normalisation in MalSourceRef.of expected: <-1> but was: <0>
shift COMPANION_SAM_LINE_IN_CLASS lands on the cast, not the signature

Pre-flight: CHECKSTYLE_EXIT=0 LICENSE_EXIT=0 TESTS_EXIT=0 BUILD_EXIT=0; license 0 invalid of 4983; 0 javadoc error: lines; 0 failing suites (MAL 1403, LAL 214, OAL 98).

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Fixes MAL operator-facing YAML attribution and generated Java debug coordinates.

Changes:

  • Adds YAML source indexing and per-stage debug line propagation.
  • Aligns generated SourceFile and LineNumberTable metadata.
  • Adds tests, documentation, and changelog updates.

Reviewed changes

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

Show a summary per file
File Description
MalFileApplier.java Stamps runtime-rule source lines.
MALDebugRecorderImpl.java Records probe source lines.
MalYamlLineIndexTest.java Tests YAML anchor indexing.
MalProbeSourceLineTest.java Tests per-stage attribution.
MalSourceMapTest.java Tests expression source mapping.
MalProbeEmissionTest.java Tests emitted probe literals.
MalPrefixInjectionRangesTest.java Tests prefix splice ranges.
MalLineAttributionTest.java Validates generated class metadata.
MalGeneratedSourceLinesTest.java Tests statement-line detection.
RuleSourceLines.java Applies source anchors to rules.
Rules.java Indexes disk-loaded rule text.
Rule.java Stores file-level anchors.
MetricsRule.java Stores per-rule anchors.
MetricRuleConfig.java Exposes source-line contracts.
MetricConvert.java Builds and propagates source maps.
MalYamlLineIndex.java Resolves YAML node lines.
MALDebugRecorder.java Adds source-line parameters.
MALDebug.java Forwards probe source lines.
DSL.java Adds source-mapped parsing.
MalSourceRef.java Models YAML/generated coordinates.
MalSourceMap.java Maps expression offsets to YAML.
MALScriptParser.java Tracks prefix and method offsets.
MALMethodChainCodegen.java Attributes chain-stage probes.
MalGeneratedSourceLines.java Finds generated statement lines.
MALExpressionModel.java Stores method-call source offsets.
MALExprCodegen.java Emits attributed probes.
MALCodegenHelper.java Adds line literals to probes.
MALClosureCodegen.java Removes misleading companion tables.
MALClassGenerator.java Aligns generated source metadata.
MALBytecodeHelper.java Builds generated-source line tables.
Analyzer.java Attributes filter/output probes.
dsl-debugging-mal.md Documents MAL sourceLine.
dynamic-code-generation-debugging.md Documents coordinate spaces.
changes.md Records the attribution fix.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@wu-sheng wu-sheng added bug Something isn't working and you are sure it's a bug! backend OAP backend related. labels Aug 8, 2026
@wu-sheng wu-sheng added this to the 11.0.0 milestone Aug 8, 2026
@wu-sheng wu-sheng changed the title Fix MAL line attribution: separate the operator and machine coordinates Fix generated-class source attribution in MAL, LAL and OAL Aug 8, 2026
@wu-sheng
wu-sheng force-pushed the feat/mal-line-attribution branch 4 times, most recently from ca01aa0 to ef199a4 Compare August 9, 2026 13:46
When generated DSL code throws, the stack frame should name the file it came
from, the line inside that file, and the line of the rule that produced it.
All three were broken, in eight places across three compilers, each producing a
number that was wrong and looked right.

  at ...rt.envoy_ca_L41_service_listener_ssl_ca_expiration_seconds.run(
         envoy_ca_L41_service_listener_ssl_ca_expiration_seconds.java:12)
                  |                                              |      |
       envoy-ca.yaml line 41                          a real file    line 12

- The _L<n>_ segment was the rules-list INDEX, not a line, so every file's
  first rule read L0. It now carries the rule's real YAML line.
- SourceFile named a file that was never written, so source-attach could not
  resolve a frame regardless of the line. It now names the sidecar on disk.
- LineNumberTable held statement ORDINALS, matching neither the YAML nor the
  generated source. MAL now maps each statement to its real sidecar line.

Fixing any two of the three leaves the frame useless, which is why the class
naming issue is part of this rather than cosmetic.

Closure companions were the gap: a companion always CLAIMED a source file,
because Javassist stamps SourceFile unconditionally, but the file was never
written -- 84 dangling references in network-profiling alone. Each now writes
its own sidecar and carries one entry at its SAM signature. One entry is
derived, not conceded: the statement scanner marks boundaries at stores to a
result slot, which a closure body never makes.

LAL and OAL had the same defect and are fixed the same way, going from 0/48
resolvable to 48/48 for LAL. Their line tables are REMOVED rather than
corrected: their statements are void invocations that store nothing, so no
sound boundary detector exists, and an ordinal that resolves is worse than an
absent attribute. A single signature entry for them is a follow-up.
Hierarchy and OAL's metrics-builder write no sidecar at all, so their
SourceFile still addresses nothing; both are recorded rather than half-fixed.

Both sidecar writers also moved off a platform-default FileWriter with a
non-ASCII header -- the pairing that produced MalformedInputException for MAL
on this branch.

Scope is deliberately the error path only. No API, no payload, no format
change: the dsl-debugging probes, records and UI are untouched, and their
line numbers come from parse-time tokens that never involved bytecode.
@wu-sheng
wu-sheng force-pushed the feat/mal-line-attribution branch from ef199a4 to 2e8df3e Compare August 9, 2026 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend OAP backend related. bug Something isn't working and you are sure it's a bug!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants