You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by "wu-sheng (via GitHub)" <gi...@apache.org> on 2026/03/12 13:18:16 UTC
[PR] LAL: introduce LogMetadata to decouple metadata from LogData in pipeline entry [skywalking]
wu-sheng opened a new pull request, #13742:
URL: https://github.com/apache/skywalking/pull/13742
### Introduce LogMetadata POJO to separate metadata from input data in LAL pipeline
Cleanly decouple metadata (service, layer, timestamp, trace context) from input data
(LogData body/tags, typed protos like Envoy ALS) at the LAL pipeline entry point.
**Key changes:**
- `LogMetadata` POJO carries service/instance/endpoint/layer/timestamp/trace context
- `LogMetadataUtils.fromLogData()` extracts metadata from `LogData` at receiver boundary
- `ExecutionContext` uses `metadata()` + `input()` instead of `log()` + `extraLog()`
- Envoy ALS receivers no longer build fake `LogData` — pass typed proto as `input` directly
- `LogSinkListener.parse()` unified to single 3-parameter signature `(LogMetadata, Object, ExecutionContext)`
- `LogTestQuery` (test tool) separated from production sink via `dryRun` flag
- Only compiler is LogData-aware (for `json{}`/`yaml{}` fallback); runtime uses untyped `ctx.input()`
- `LalRuntimeHelper.tagValue()` casts directly to `LogData.Builder` (no instanceof guard — only called by compiled code)
- [ ] If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #<issue number>.
- [ ] Update the [`CHANGES` log](https://github.com/apache/skywalking/blob/master/docs/en/changes/changes.md).
--
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] LAL: introduce LogMetadata to decouple metadata from LogData in pipeline entry [skywalking]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #13742:
URL: https://github.com/apache/skywalking/pull/13742#discussion_r2924745211
##########
oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/spi/LALSourceTypeProvider.java:
##########
@@ -23,8 +23,8 @@
* SPI for receiver plugins to declare the input and default output types for
* LAL rules on a specific {@link Layer}.
*
- * <p><b>Input type</b> ({@link #inputType()}): The Java type of the {@code extraLog}
- * passed to LAL via {@code ILogAnalyzerService.doAnalysis(LogData, Message)}.
+ * <p><b>Input type</b> ({@link #inputType()}): The Java type of the {@code input}
+ * passed to LAL via {@code ILogAnalyzerService.doAnalysis(LogMetadata, Object)}.
* The LAL compiler uses this at compile time to generate optimized direct
* getter calls for {@code parsed.*} field access. This is per-layer because
* all rules for a layer share the same input proto type.
Review Comment:
Fixed — updated javadoc fallback from `LogData.Builder` to `LogMetadata`.
##########
oap-server/analyzer/log-analyzer/CLAUDE.md:
##########
@@ -182,10 +182,11 @@ The generator detects the parser type from the AST at compile time and generates
| JSON/YAML | `parsed.service` | `h.mapVal("service")` |
| JSON/YAML nested | `parsed.a.b` | `h.mapVal("a", "b")` |
| TEXT (regexp) | `parsed.level` | `h.group("level")` |
-| NONE + inputType | `parsed.response.code` | `((ExtraLogType) h.ctx().extraLog()).getResponse().getCode()` |
-| NONE + no inputType | `parsed.service` | `h.ctx().log().getService()` (LogData.Builder fallback) |
-| log fields | `log.service` | `h.ctx().log().getService()` |
-| log trace | `log.traceContext.traceId` | `h.ctx().log().getTraceContext().getTraceId()` |
+| NONE + inputType | `parsed.response.code` | `((InputType) h.ctx().input()).getResponse().getCode()` |
+| NONE + no inputType | `parsed.service` | `h.ctx().metadata().getService()` (LogMetadata fallback) |
+| log fields (metadata) | `log.service` | `h.ctx().metadata().getService()` |
+| log fields (LogData) | `log.body` | `h.ctx().log().getBody()` |
Review Comment:
Fixed — updated to `((LogData.Builder) h.ctx().input()).getBody()`.
##########
test/script-cases/script-runtime-with-groovy/mal-lal-v1-v2-checker/src/test/java/org/apache/skywalking/oap/server/checker/lal/LalBenchmark.java:
##########
@@ -239,12 +239,14 @@ public void executeV1(final Blackhole bh) {
public void executeV2(final Blackhole bh) {
for (int i = 0; i < v2Exprs.size(); i++) {
try {
+ final LogData logData = testLogs.get(i);
+ final Message extraLog = extraLogs.get(i);
+ final org.apache.skywalking.oap.server.core.source.LogMetadata metadata =
+ org.apache.skywalking.oap.server.core.source.LogMetadataUtils.fromLogData(logData);
+ final Object input = extraLog != null ? extraLog : logData;
final org.apache.skywalking.oap.log.analyzer.v2.dsl.ExecutionContext ctx =
- new org.apache.skywalking.oap.log.analyzer.v2.dsl.ExecutionContext()
- .log(testLogs.get(i));
- if (extraLogs.get(i) != null) {
- ctx.extraLog(extraLogs.get(i));
- }
+ new org.apache.skywalking.oap.log.analyzer.v2.dsl.ExecutionContext();
+ ctx.init(metadata, input);
Review Comment:
Fixed — now passes `logData.toBuilder()` for v2 input.
##########
oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/source/LogMetadataUtils.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.skywalking.oap.server.core.source;
+
+import org.apache.skywalking.apm.network.logging.v3.LogData;
+import org.apache.skywalking.apm.network.logging.v3.TraceContext;
+
+/**
+ * Utility for building {@link LogMetadata} from protobuf {@link LogData}.
+ */
+public final class LogMetadataUtils {
+
+ private LogMetadataUtils() {
+ }
+
+ /**
+ * Build LogMetadata from a LogData protobuf message.
+ */
+ public static LogMetadata fromLogData(final LogData logData) {
+ LogMetadata.TraceContext tc = null;
+ if (logData.hasTraceContext()) {
+ final TraceContext ctxProto =
+ logData.getTraceContext();
+ tc = LogMetadata.TraceContext.builder()
+ .traceId(ctxProto.getTraceId())
+ .traceSegmentId(ctxProto.getTraceSegmentId())
+ .spanId(ctxProto.getSpanId())
+ .build();
+ }
+ return LogMetadata.builder()
+ .service(logData.getService())
+ .serviceInstance(logData.getServiceInstance())
+ .endpoint(logData.getEndpoint())
+ .layer(logData.getLayer())
+ .timestamp(logData.getTimestamp())
+ .traceContext(tc)
+ .build();
+ }
+
+ /**
+ * Build LogMetadata from a LogData.Builder.
+ */
+ public static LogMetadata fromLogData(final LogData.Builder builder) {
+ return fromLogData(builder.build());
Review Comment:
Fixed — now extracts fields directly from the Builder without calling `build()`.
--
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] LAL: introduce LogMetadata to decouple metadata from LogData in pipeline entry [skywalking]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13742:
URL: https://github.com/apache/skywalking/pull/13742#discussion_r2924678237
##########
oap-server/analyzer/log-analyzer/CLAUDE.md:
##########
@@ -182,10 +182,11 @@ The generator detects the parser type from the AST at compile time and generates
| JSON/YAML | `parsed.service` | `h.mapVal("service")` |
| JSON/YAML nested | `parsed.a.b` | `h.mapVal("a", "b")` |
| TEXT (regexp) | `parsed.level` | `h.group("level")` |
-| NONE + inputType | `parsed.response.code` | `((ExtraLogType) h.ctx().extraLog()).getResponse().getCode()` |
-| NONE + no inputType | `parsed.service` | `h.ctx().log().getService()` (LogData.Builder fallback) |
-| log fields | `log.service` | `h.ctx().log().getService()` |
-| log trace | `log.traceContext.traceId` | `h.ctx().log().getTraceContext().getTraceId()` |
+| NONE + inputType | `parsed.response.code` | `((InputType) h.ctx().input()).getResponse().getCode()` |
+| NONE + no inputType | `parsed.service` | `h.ctx().metadata().getService()` (LogMetadata fallback) |
+| log fields (metadata) | `log.service` | `h.ctx().metadata().getService()` |
+| log fields (LogData) | `log.body` | `h.ctx().log().getBody()` |
Review Comment:
The table entry for `log.body` shows generated code `h.ctx().log().getBody()`, but `ExecutionContext` no longer has `log()`. The codegen in this PR uses `((LogData.Builder) h.ctx().input()).getBody()` for LogData-only fields, so this doc example should be updated to match the new runtime API.
```suggestion
| log fields (LogData) | `log.body` | `((LogData.Builder) h.ctx().input()).getBody()` |
```
##########
test/script-cases/script-runtime-with-groovy/mal-lal-v1-v2-checker/src/test/java/org/apache/skywalking/oap/server/checker/lal/LalBenchmark.java:
##########
@@ -239,12 +239,14 @@ public void executeV1(final Blackhole bh) {
public void executeV2(final Blackhole bh) {
for (int i = 0; i < v2Exprs.size(); i++) {
try {
+ final LogData logData = testLogs.get(i);
+ final Message extraLog = extraLogs.get(i);
+ final org.apache.skywalking.oap.server.core.source.LogMetadata metadata =
+ org.apache.skywalking.oap.server.core.source.LogMetadataUtils.fromLogData(logData);
+ final Object input = extraLog != null ? extraLog : logData;
final org.apache.skywalking.oap.log.analyzer.v2.dsl.ExecutionContext ctx =
- new org.apache.skywalking.oap.log.analyzer.v2.dsl.ExecutionContext()
- .log(testLogs.get(i));
- if (extraLogs.get(i) != null) {
- ctx.extraLog(extraLogs.get(i));
- }
+ new org.apache.skywalking.oap.log.analyzer.v2.dsl.ExecutionContext();
+ ctx.init(metadata, input);
Review Comment:
In the v2 benchmark path, `input` is set to `logData` (an immutable `LogData`) when there is no `extraLog`. The v2 runtime/codegen assumes standard-log input is a `LogData.Builder` (e.g., `log.body`, `json{}`, `tag()` all cast `ctx.input()` to `LogData.Builder`), so this will trigger `ClassCastException` and the benchmark will silently ignore it due to the broad catch, skewing results. Consider passing `logData.toBuilder()` (or storing builders in `testLogs`) for the non-ALS case.
##########
oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/spi/LALSourceTypeProvider.java:
##########
@@ -23,8 +23,8 @@
* SPI for receiver plugins to declare the input and default output types for
* LAL rules on a specific {@link Layer}.
*
- * <p><b>Input type</b> ({@link #inputType()}): The Java type of the {@code extraLog}
- * passed to LAL via {@code ILogAnalyzerService.doAnalysis(LogData, Message)}.
+ * <p><b>Input type</b> ({@link #inputType()}): The Java type of the {@code input}
+ * passed to LAL via {@code ILogAnalyzerService.doAnalysis(LogMetadata, Object)}.
* The LAL compiler uses this at compile time to generate optimized direct
* getter calls for {@code parsed.*} field access. This is per-layer because
* all rules for a layer share the same input proto type.
Review Comment:
The updated Javadoc here references the new `(LogMetadata, Object)` input, but later in this same comment block the “Input type resolution order” still lists `LogData.Builder` as the final fallback. With the changes in this PR, the fallback for no-parser/no-inputType `parsed.*` access is `LogMetadata`, so the Javadoc should be updated for consistency.
##########
oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/source/LogMetadataUtils.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.skywalking.oap.server.core.source;
+
+import org.apache.skywalking.apm.network.logging.v3.LogData;
+import org.apache.skywalking.apm.network.logging.v3.TraceContext;
+
+/**
+ * Utility for building {@link LogMetadata} from protobuf {@link LogData}.
+ */
+public final class LogMetadataUtils {
+
+ private LogMetadataUtils() {
+ }
+
+ /**
+ * Build LogMetadata from a LogData protobuf message.
+ */
+ public static LogMetadata fromLogData(final LogData logData) {
+ LogMetadata.TraceContext tc = null;
+ if (logData.hasTraceContext()) {
+ final TraceContext ctxProto =
+ logData.getTraceContext();
+ tc = LogMetadata.TraceContext.builder()
+ .traceId(ctxProto.getTraceId())
+ .traceSegmentId(ctxProto.getTraceSegmentId())
+ .spanId(ctxProto.getSpanId())
+ .build();
+ }
+ return LogMetadata.builder()
+ .service(logData.getService())
+ .serviceInstance(logData.getServiceInstance())
+ .endpoint(logData.getEndpoint())
+ .layer(logData.getLayer())
+ .timestamp(logData.getTimestamp())
+ .traceContext(tc)
+ .build();
+ }
+
+ /**
+ * Build LogMetadata from a LogData.Builder.
+ */
+ public static LogMetadata fromLogData(final LogData.Builder builder) {
+ return fromLogData(builder.build());
Review Comment:
`fromLogData(LogData.Builder)` currently calls `builder.build()` and then re-reads fields from the built message. In hot receiver paths this duplicates the entire `LogData` (including potentially large body/tags) just to extract metadata fields. Consider extracting directly from the builder (and its traceContext builder/message) without building a full `LogData` instance.
```suggestion
LogMetadata.TraceContext tc = null;
if (builder.hasTraceContext()) {
final TraceContext.Builder ctxBuilder = builder.getTraceContextBuilder();
tc = LogMetadata.TraceContext.builder()
.traceId(ctxBuilder.getTraceId())
.traceSegmentId(ctxBuilder.getTraceSegmentId())
.spanId(ctxBuilder.getSpanId())
.build();
}
return LogMetadata.builder()
.service(builder.getService())
.serviceInstance(builder.getServiceInstance())
.endpoint(builder.getEndpoint())
.layer(builder.getLayer())
.timestamp(builder.getTimestamp())
.traceContext(tc)
.build();
```
--
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] LAL: introduce LogMetadata to decouple metadata from LogData in pipeline entry [skywalking]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng merged PR #13742:
URL: https://github.com/apache/skywalking/pull/13742
--
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