[FLINK-38353][spans] FLIP-483: Add support for children Spans #26985

pnowojski · 2025-09-12T15:19:36Z

What is the purpose of the change

https://cwiki.apache.org/confluence/display/FLINK/FLIP-483%3A+Add+support+for+children+Spans

Brief change log

Please check individual commit messages.

Verifying this change

Adds a couple of unit tests and extends the existing ITCase

Does this pull request potentially affect one of the following parts:

Dependencies (does it add or upgrade a dependency): (yes / no)
The public API, i.e., is any changed class annotated with @Public(Evolving): (yes / no)
The serializers: (yes / no / don't know)
The runtime per-record code paths (performance sensitive): (yes / no / don't know)
Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (yes / no / don't know)
The S3 file system connector: (yes / no / don't know)

Documentation

Does this pull request introduce a new feature? (yes / no)
If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)

flinkbot · 2025-09-12T15:26:26Z

CI report:

3f92c0c Azure: FAILURE

Bot commands

The @flinkbot bot supports the following commands:

@flinkbot run azure re-run the last Azure build

Efrat19 · 2025-09-14T12:09:54Z

flink-core/src/main/java/org/apache/flink/configuration/TraceOptions.java

+                            .defaultValue(CheckpointSpanDetailLevel.SPANS_PER_CHECKPOINT)
+                            .withDescription(
+                                    "Detail level for reporting checkpoint spans. Possible values:\n"
+                                            + "- SPAN_PER_CHECKPOINT (default): Single span per checkpoint. Aggregated sum/max for submetrics from all tasks and subtasks per checkpoint\n"


Suggested change

+ "- SPAN_PER_CHECKPOINT (default): Single span per checkpoint. Aggregated sum/max for submetrics from all tasks and subtasks per checkpoint\n"

+ "- SPANS_PER_CHECKPOINT (default): Single span per checkpoint. Aggregated sum/max for submetrics from all tasks and subtasks per checkpoint\n"

Same for a few more times across the description

@Efrat19 in the case of SPAN_PER_CHECKPOINT there is only one span so singular makes sense top me. The others could have multiple so are SPANS.

SPANS_PER_TASK will have a SPAN for each task plus one for the PARENT, so is a bit misleading. We could say
CHILDEN_SPANS_PER_TASK and CHILDEN_SPANS_PER_SUBTASK if we think this is clearer. WDYT @pnowojski ? I do not have strong views to change this, but thought I would bring it to your attention.

Yeah, lets keep the first two singular other plural with CHILDREN_SPANS

I have also re-used constants to avoid problems with names getting out of sync.

Efrat19 · 2025-09-14T12:23:11Z

...link-metrics-otel/src/main/java/org/apache/flink/traces/otel/OpenTelemetryTraceReporter.java

-        // TODO: not yet supported
-        // for (Span childSpan : span.getChildren()) {
-        //    notifyOfAddedSpanInternal(childSpan, currentOtelSpan);
-        // }


Nit: remove comment? (code is identical to the loop below)

It has been removed?

RocMarshal · 2025-09-18T11:01:11Z

flink-core/src/main/java/org/apache/flink/configuration/TraceOptions.java

+                            .withDescription(
+                                    String.format(
+                                            "Detail level for reporting checkpoint spans. Possible values:\n"
+                                                    + "- %s (default): Single span per checkpoint. Aggregated sum/max for submetrics from all tasks and subtasks per checkpoint\n"
+                                                    + "- %s: Single span per checkpoint. Same as %s, plus arrays of aggregated values per task.\n"
+                                                    + "- %s: Same as %s plus children spans per each task. Each task span with aggregated sum/max submetrics from subtasks.\n"
+                                                    + "- %s: Same as %s plus children spans per each subtask. Child spans for tasks and grand-child spans for subtasks.",
+                                            CheckpointSpanDetailLevel.SPAN_PER_CHECKPOINT.name(),
+                                            CheckpointSpanDetailLevel.SPAN_PER_CHECKPOINT_WITH_TASKS
+                                                    .name(),
+                                            CheckpointSpanDetailLevel.SPAN_PER_CHECKPOINT.name(),
+                                            CheckpointSpanDetailLevel.CHILDREN_SPANS_PER_TASK
+                                                    .name(),
+                                            CheckpointSpanDetailLevel.SPAN_PER_CHECKPOINT.name(),
+                                            CheckpointSpanDetailLevel.CHILDREN_SPANS_PER_SUBTASK
+                                                    .name(),
+                                            CheckpointSpanDetailLevel.CHILDREN_SPANS_PER_TASK


Could we use the APIs 'Description.builder().list() ... ' to construct the description ?
Such as PipelineOptions#SERIALIZATION_CONFIG...

Thanks for the pointer, I was not aware of that utility.

flink-core/src/main/java/org/apache/flink/configuration/TraceOptions.java

flink-metrics/flink-metrics-core/src/main/java/org/apache/flink/traces/SimpleSpan.java

...ime/src/test/java/org/apache/flink/runtime/checkpoint/DefaultCheckpointStatsTrackerTest.java

RocMarshal · 2025-09-18T11:06:21Z

...ime/src/test/java/org/apache/flink/runtime/checkpoint/DefaultCheckpointStatsTrackerTest.java

+        assertThat(reportedEvents.size()).isEqualTo(1);
+
+        Map<String, Object> expected = new HashMap<>();
+        expected.put("checkpointId", 42L);


Could this constants be extracted as immutable strings?

One advantage of doing so is that it avoids using them directly in multiple places.

Of course, such a change might come at the cost of some readability.

I'm not sure if this is worth here due to the sheer amount of the constants 🤔

Thanks.

Could this constants be extracted as immutable strings?

Just a trivial comment. Please ignore it.

As proposed and described in: https://cwiki.apache.org/confluence/display/FLINK/FLIP-483%3A+Add+support+for+children+Spans

As proposed in https://cwiki.apache.org/confluence/display/FLINK/FLIP-483%3A+Add+support+for+children+Spans

pnowojski changed the title ~~FLIP-483: Add support for children Spans~~ [FLINK-38353][spans] FLIP-483: Add support for children Spans Sep 12, 2025

Efrat19 reviewed Sep 14, 2025

View reviewed changes

github-actions bot added the community-reviewed PR has been reviewed by the community. label Sep 14, 2025

pnowojski force-pushed the f38353 branch from 5ae8b55 to 94b2968 Compare September 17, 2025 13:10

pnowojski requested a review from Efrat19 September 17, 2025 13:10

RocMarshal reviewed Sep 18, 2025

View reviewed changes

StefanRRichter and others added 3 commits September 19, 2025 09:50

[FLINK-38353][spans] Add support for nested spans to metrics reporting.

7296718

As proposed and described in: https://cwiki.apache.org/confluence/display/FLINK/FLIP-483%3A+Add+support+for+children+Spans

[FLINK-38353][spans] Report checkpoint sub spans

cbb64ac

As proposed in https://cwiki.apache.org/confluence/display/FLINK/FLIP-483%3A+Add+support+for+children+Spans

[FLINK-38353][docs] Update docs for children spans

3f92c0c

pnowojski force-pushed the f38353 branch from 94b2968 to 3f92c0c Compare September 19, 2025 07:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[FLINK-38353][spans] FLIP-483: Add support for children Spans #26985

[FLINK-38353][spans] FLIP-483: Add support for children Spans #26985

Uh oh!

pnowojski commented Sep 12, 2025

Uh oh!

flinkbot commented Sep 12, 2025 •

edited

Loading

Uh oh!

Efrat19 Sep 14, 2025

Uh oh!

davidradl Sep 15, 2025

Uh oh!

pnowojski Sep 17, 2025 •

edited

Loading

Uh oh!

pnowojski Sep 17, 2025

Uh oh!

Efrat19 Sep 14, 2025

Uh oh!

pnowojski Sep 17, 2025

Uh oh!

RocMarshal Sep 18, 2025 •

edited

Loading

Uh oh!

pnowojski Sep 19, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

RocMarshal Sep 18, 2025

Uh oh!

pnowojski Sep 19, 2025

Uh oh!

RocMarshal Sep 19, 2025

Uh oh!

Uh oh!

	+ "- SPAN_PER_CHECKPOINT (default): Single span per checkpoint. Aggregated sum/max for submetrics from all tasks and subtasks per checkpoint\n"
	+ "- SPANS_PER_CHECKPOINT (default): Single span per checkpoint. Aggregated sum/max for submetrics from all tasks and subtasks per checkpoint\n"

[FLINK-38353][spans] FLIP-483: Add support for children Spans #26985

Are you sure you want to change the base?

[FLINK-38353][spans] FLIP-483: Add support for children Spans #26985

Uh oh!

Conversation

pnowojski commented Sep 12, 2025

What is the purpose of the change

Brief change log

Verifying this change

Does this pull request potentially affect one of the following parts:

Documentation

Uh oh!

flinkbot commented Sep 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI report:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pnowojski Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RocMarshal Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

flinkbot commented Sep 12, 2025 •

edited

Loading

pnowojski Sep 17, 2025 •

edited

Loading

RocMarshal Sep 18, 2025 •

edited

Loading