You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by "wankai123 (via GitHub)" <gi...@apache.org> on 2026/07/21 02:31:32 UTC

[PR] Route LAL rules within a layer by input type; add envoy-als-tcp rule [skywalking]

wankai123 opened a new pull request, #13955:
URL: https://github.com/apache/skywalking/pull/13955

   ### Route LAL rules within a layer by input type; add `envoy-als-tcp`
   
   LAL rules keyed to the same telemetry layer are all evaluated against every log of
   that layer. When a layer carries more than one proto input shape this breaks: the
   generated `parsed.*` code casts the input unconditionally, so a rule compiled for one
   proto throws `ClassCastException` on another. On `Layer.MESH` both HTTP and TCP Envoy
   access logs (and network-profiling `LogData`) share the bucket, so the HTTP `envoy-als`
   rule already throws — caught in `LogFilterListener.build()` and logged per log — whenever
   a TCP entry or a network-profiling log arrives.
   
   This PR makes rule dispatch input-type aware:
   
   - Each compiled rule now carries its **effective input type** — the proto class its
     `parsed.*` getters cast to, or `null` for parser-based (`json`/`yaml`/`text`) and
     untyped rules, which continue to run against any input. Derived once at compile time
     in `LALClassGenerator` and threaded onto the `DSL`.
   - `LogFilterListener.parse()` skips any rule whose declared input type does not match the
     incoming object (`inputType.isInstance(input)`), so each entry only reaches the rules
     meant for it. This removes the latent `ClassCastException` and the per-log WARN spam.
   - Routing keys off the **effective** type, not the resolved one, so a parser-based rule
     such as `network-profiling-slow-trace` (whose SPI-resolved type is `HTTPAccessLogEntry`
     but which reads a parsed map) is never wrongly filtered.
   
   Adds an `envoy-als-tcp` rule (`inputType: io.envoyproxy.envoy.data.accesslog.v3.TCPAccessLogEntry`)
   alongside the existing HTTP `envoy-als`; both share `Layer.MESH` and each now only sees
   its own entry type. TCP entries have no HTTP status code, so the rule filters on
   `commonProperties.responseFlags` and samples by `service:responseFlags`.
   
   ### New feature / behavior
   - [x] This builds on the existing LAL per-rule `inputType` + `LALSourceTypeProvider` SPI
     mechanism (documented in `oap-server/analyzer/log-analyzer/CLAUDE.md`); no separate design doc.
   - [x] Update the documentation to include this new feature. (CHANGES log)
   - [x] Tests (UT) are added to verify the new feature.
   - [x] Not UI related.
   
   ### Also fixes a latent bug
   - [x] Added unit tests reproducing the mixed-input-type dispatch (`LogFilterListenerRoutingTest`):
     an HTTP entry reaches only the HTTP + parser rules, a TCP entry only the TCP + parser rules,
     no cross-type `ClassCastException`.
   - [x] Root cause: `parsed.*` codegen emits an unguarded proto cast, and every rule in a layer
     runs against every input; fix routes by the rule's effective input type before evaluation.
   
   ### Tests added
   - `LogFilterListenerRoutingTest` — routing selection (3 cases).
   - `LALClassGeneratorExtractorTest` — `effectiveInputType` derivation (parser ⇒ `null`) and a
     compile guard for the shipped `envoy-als-tcp` rule against `TCPAccessLogEntry`.
   - `envoy-als-tcp` data-driven execution cases in `LALScriptExecutionTest` (abnormal ⇒ tag +
     save; normal ⇒ abort).
   
   - [ ] If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #<issue number>.
   - [x] Update the [`CHANGES` log](https://github.com/apache/skywalking/blob/master/docs/en/changes/changes.md).
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Route LAL rules within a layer by input type; add envoy-als-tcp rule [skywalking]

Posted by "wankai123 (via GitHub)" <gi...@apache.org>.
wankai123 merged PR #13955:
URL: https://github.com/apache/skywalking/pull/13955


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Route LAL rules within a layer by input type; add envoy-als-tcp rule [skywalking]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13955:
URL: https://github.com/apache/skywalking/pull/13955#discussion_r3619039719


##########
oap-server/analyzer/log-analyzer/src/test/resources/scripts/lal/test-lal/oap-cases/envoy-als.data.yaml:
##########
@@ -47,6 +47,29 @@ envoy-als:
       abort: false
       tag.response.flag: "upstream_connection_failure: true\n"
 
+# The envoy-als-tcp rule processes protobuf TCPAccessLogEntry as extraLog.
+# TCP entries have no HTTP status code; the rule filters on
+# commonProperties.responseFlags and tags the connection byte counts.

Review Comment:
   The comment says the TCP rule "tags the connection byte counts", but the envoy-als-tcp DSL only tags `response.flag` (responseFlags). This is misleading for future maintenance of the data-driven cases.



##########
oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/dsl/DSL.java:
##########
@@ -47,6 +47,15 @@ public class DSL {
     @Getter
     private final LalExpression expression;
     private final FilterSpec filterSpec;
+    /**
+     * Effective proto input type this rule casts to, or {@code null} for
+     * parser-based / untyped rules (which run against any input). The runtime
+     * skips a rule whose declared type doesn't match the incoming log — this
+     * is how HTTP and TCP envoy access logs, which share {@code Layer.MESH},
+     * route to their own rules without cross-type {@code ClassCastException}.
+     */
+    @Getter
+    private final Class<?> inputType;

Review Comment:
   `DSL#getInputType()` now returns the *effective* input type (null when a parser is present), which differs from the declared/resolved input type coming from YAML/SPI. Keeping the field/getter name as `inputType` makes it easy to misread and accidentally use it as the declared type in future code; consider renaming to `effectiveInputType` (and optionally retaining the declared type separately if needed for debugging).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org