Add labels to sys.server #18547

GabrielCWT · 2025-09-19T01:21:52Z

This PR adds support for labels being configured in the config file. It uses the key druid.labels and expects the value to be a JSON structure of key-value pairs.
The labels will be displayed in the web-console under the "Services" tab.

Example: druid.labels={"broker-label":"broker-value","location": "Airtrunk"}

Below is a screenshot of the implementation with different labels for different node types. Note that some columns have been hidden to capture the important details.

FrankChen021 · 2025-09-19T02:12:53Z

Since the label is defined as a Map internally, will these configuration work?

druid.label.key1 = val1
druid.label.key2 = val2

GabrielCWT · 2025-09-19T03:08:13Z

Since the label is defined as a Map internally, will these configuration work?
druid.label.key1 = val1
druid.label.key2 = val2

You are right, I have updated the docs to reflect that both formats are acceptable

kfaraz · 2025-09-19T04:50:19Z

@GabrielCWT , how do you intend to use these labels?
In particular, do they need to be displayed on the web-console and/or be a part of the DruidNode object?

GabrielCWT · 2025-09-19T07:05:40Z

@GabrielCWT , how do you intend to use these labels? In particular, do they need to be displayed on the web-console and/or be a part of the DruidNode object?

As of now, the use case is to display the IDC location of the service. Putting it in the web console would make it easier for us to see the locations of all services.

This feature is similar to the kubernetes label feature which helps to add identifying attributes to each service.

I felt that since it is attached to each service, we should store it in the DruidNode. I am open to suggestions if you feel that this should be stored in a different place.

server/src/main/java/org/apache/druid/server/DruidNode.java

sql/src/main/java/org/apache/druid/sql/calcite/schema/SystemSchema.java

kfaraz · 2025-09-22T12:04:31Z

Thanks for sharing the details, @GabrielCWT ! That sounds like a reasonable use case.

My only concern was with adding a new type of free-form info to DruidNode since it is primarily used for service discovery.
There can be issues with Zookeeper if a node ends up having very large label values,
but I suppose it is not very likely to happen and would be a configuration issue rather than a code issue anyway.

The alternative would be to just bind it to a completely new config object and then it would be served over the /status/properties API but then you wouldn't be able to display it on the web-console.

GabrielCWT · 2025-09-23T02:05:40Z

My only concern was with adding a new type of free-form info to DruidNode since it is primarily used for service discovery. There can be issues with Zookeeper if a node ends up having very large label values, but I suppose it is not very likely to happen and would be a configuration issue rather than a code issue anyway.

This was indeed one of my concerns as well but right now the use case will be a maximum of 2-3 values. As you mentioned, an absurdly large number of label values would be a configuration issue.

The alternative would be to just bind it to a completely new config object and then it would be served over the /status/properties API but then you wouldn't be able to display it on the web-console.

The main idea is for it to be visible on the web-console and therefore I think sticking to the current implementation is better.

FrankChen021 · 2025-09-23T03:17:48Z

Thanks for sharing the details, @GabrielCWT ! That sounds like a reasonable use case.

My only concern was with adding a new type of free-form info to DruidNode since it is primarily used for service discovery. There can be issues with Zookeeper if a node ends up having very large label values, but I suppose it is not very likely to happen and would be a configuration issue rather than a code issue anyway.

The alternative would be to just bind it to a completely new config object and then it would be served over the /status/properties API but then you wouldn't be able to display it on the web-console.

I think it's better to show the label in the web console. Another use case is that we can inject some container meta data like container name to this property when druid is deployed in K8s, so that we know which service/host maps to which container from web-console

kfaraz · 2025-09-23T03:20:49Z

I think it's better to show the label in the web console. Another use case is that we can inject some container meta data like container name to this property when druid is deployed in K8s, so that we know which service/host maps to which container from web-console

Yeah, I agree. Labels can have several uses. I was only thinking out loud.
I prefer the current approach myself as long as we don't put unnecessarily large values in the labels.

docs/querying/sql-metadata-tables.md

kfaraz · 2025-09-23T08:50:34Z

server/src/main/java/org/apache/druid/server/DruidNode.java

  );

+  @JsonProperty
+  private Map<String, String> labels;


Suggested change

private Map<String, String> labels;

private final Map<String, String> labels;

Currently all variables are being assigned in the init function, this prevents the object's variables from being final. I will be following this convention and as such not be assigning final to the variable

Ah, I missed the init() call. final is always better for such bean fields.
But you are right, we cannot do it in this PR.

server/src/main/java/org/apache/druid/server/DruidNode.java

server/src/test/java/org/apache/druid/server/DruidNodeTest.java

kfaraz · 2025-09-24T02:51:51Z

sql/src/main/java/org/apache/druid/sql/calcite/schema/SystemSchema.java

      .add("max_size", ColumnType.LONG)
      .add("is_leader", ColumnType.LONG)
      .add("start_time", ColumnType.STRING)
+      .add("labels", ColumnType.NESTED_DATA)


I don't think any of the other columns use nested data as the type.
Even columns in sys.segments like last_compaction_state, dimensions, metrics, etc
use STRING as the type. I think we should just stick to that.

Not sure if I misunderstood our previous discussion but the issue I was facing was that when it was a STRING type, the data being returned for the Map was in the form "labels": "{brokerTest=myValue, brokerTest2=myValue2}", therefore I used jsonMapper to serialise the labels.

We then agreed to return it in JSON format thus changing it to NESTED_DATA type. Just want to confirm that you want to revert it back to the original implementation

node.getLabels() == null ? null : jsonMapper.writeValueAsString(node.getLabels())

Oh, I see what's happening here. Sorry, I had missed this in the initial pass.
You will need to use a jsonMapper and keep the column type as STRING
since that is what SegmentsTable is doing as well but lazilly.

The segments table uses a list of SEGMENTS_JSON_FIELDS to identify which fields to serialize as json.
Ideally, you would want to do something similar to avoid invoking jsonMapper.writeValueAsString() every time
we receive a query for sys.servers so that we serialize that field only when needed.

But for now, you may just stick to what you had originally that is:
node.getLabels() == null ? null : jsonMapper.writeValueAsString(node.getLabels())

The perf improvement can be done later.

Sorry for the confusion 😅 !

It seems that the optimization would also require making ServersTable implement ProjectableFilterableTable instead of ScannableTable. Not sure if that change would have any other impact.

I don't think this optimization helps. because on the web-console, the label is always selected, this is the most scenario that sys.server table is queried.

Considering the number of servers is not very large(and even the number is huge, at the early phase of this feature, it may not be widely used), the serialization here might NOT be a problem.

If we're going to optimize the serialization in future, my suggestion is to cache the serialized value in the DruidNode.

I will open up an issue for a possible refactor to implement ProjectableFilterableTable. It would be useful if the number of fields requiring serialising increases. For now I will stick with what was written originally.

If we're going to optimize the serialization in future, my suggestion is to cache the serialized value in the DruidNode.

This was also an optimisation I was thinking of. What are your thoughts? @kfaraz

for the serialization problem, I think the fundament problem here is that JSON is not supported by calcite. Not sure if there's a way to make it at the calcite side.

If we're going to optimize the serialization in future, my suggestion is to cache the serialized value in the DruidNode.

Yeah, I suppose that is possible by passing a @JacksonInject ObjectMapper into the DruidNode constructor.
But it really seems unnecessary at this point.

@FrankChen021 , as you mention, serializing a small map is not a costly operation, we need not worry about that optimization right now.
I should think that there are many more costlier serialization steps involved in a single SELECT * FROM sys.servers query.
If the labels map becomes large, then we have other problems as mentioned before anyway.

I don't think this optimization helps. because on the web-console, the label is always selected, this is the most scenario that sys.server table is queried.

Considering the number of servers is not very large(and even the number is huge, at the early phase of this feature, it may not be widely used), the serialization here might NOT be a problem.

+1

sql/src/test/java/org/apache/druid/sql/calcite/schema/SystemSchemaTest.java

website/.spelling

kfaraz · 2025-09-24T02:55:09Z

@vogievetsky, requested your review since this PR also touches multiple web-console files.

FrankChen021

LGTM

kfaraz

Left some final non-blocking comments.

+1 after CI passes.

Best to get an approval from @vogievetsky too before merging this off.

kfaraz · 2025-09-24T08:06:51Z

sql/src/main/java/org/apache/druid/sql/calcite/schema/SystemSchema.java

-          isLeader ? 1L : 0L,
-          toStringOrNull(discoveryDruidNode.getStartTime())
-      };
+      try {


Rather than adding the try-catch in the 2 places, better to add a new method in JacksonUtils named writeValueAsString() which catches the JsonProcessingException and throws a DruidException instead. It will be useful for other places in the code too.

Here is my initial implementation, let me know if you have any comments about it.

public static String writeValueAsString(ObjectMapper jsonMapper, Object value) throws DruidException { try { return jsonMapper.writeValueAsString(value); } catch (JsonProcessingException e) { throw InvalidInput.exception(e, "Failed to serialize object as JSON"); } }

throw InternalServerError.exception() since we don't know exactly why the serialization failed.

sql/src/test/java/org/apache/druid/sql/calcite/schema/SystemSchemaTest.java

FrankChen021 · 2025-10-09T01:49:01Z

Hi @vogievetsky
I would like this one to be merged in 35. If you're too busy to review the changes, I will merge it first.

GabrielCWT added 2 commits September 19, 2025 09:19

Add labels to servers

b2d8bb8

Add labels to FE

71e549f

github-actions bot added Area - Documentation Area - Querying Area - Web Console labels Sep 19, 2025

Fix missing assignment to labels variable

7dee2ca

GabrielCWT marked this pull request as ready for review September 19, 2025 01:39

Update docs

1c24c13

FrankChen021 reviewed Sep 22, 2025

View reviewed changes

server/src/main/java/org/apache/druid/server/DruidNode.java Outdated Show resolved Hide resolved

sql/src/main/java/org/apache/druid/sql/calcite/schema/SystemSchema.java Outdated Show resolved Hide resolved

GabrielCWT added 2 commits September 23, 2025 10:12

Remove labels default value

22de456

Return labels as JSON object instead of string

da63bf9

GabrielCWT added 7 commits September 23, 2025 14:09

Fix checkstyle

69a6025

Update tests

8113e17

Revert Intellij change

adb0f92

Update spelling

34e5656

Update IT json

71fddfb

Fix format for scss

440be3b

Update snapshot

0b8e2c2

kfaraz reviewed Sep 24, 2025

View reviewed changes

kfaraz requested a review from vogievetsky September 24, 2025 02:54

FrankChen021 approved these changes Sep 24, 2025

View reviewed changes

GabrielCWT added 2 commits September 24, 2025 15:00

Fix based on comments

fec3d61

Add DruidNode tests

466e00f

kfaraz approved these changes Sep 24, 2025

View reviewed changes

GabrielCWT added 7 commits September 24, 2025 16:46

Refactor to use JacksonUtils.writeValueAsString

01f49f0

Remove unused case for columnType

b722389

Update services-view to expect string return for labels

ff9bed7

Throw InternalServerError instead of InvalidInput

8ae4dfd

Merge branch 'master' into apachegh-82-add-label

b0cc6b6

Update snapshot and format

89d4d19

Update test with new row size

b03bc3d

FrankChen021 added this to the 35.0.0 milestone Oct 8, 2025

GabrielCWT added 2 commits October 8, 2025 11:12

Hide labels during aggregation

5cda052

Merge branch 'master' into apachegh-82-add-label

632be3e

Merge branch 'master' into apachegh-82-add-label

2261145

FrankChen021 merged commit d2f5f69 into apache:master Oct 11, 2025
102 of 104 checks passed

cecemei pushed a commit to cecemei/druid that referenced this pull request Oct 17, 2025

Add labels to sys.server (apache#18547)

514a63a

cecemei mentioned this pull request Oct 17, 2025

[Backport] Add labels to sys.server #18652

Merged

cecemei pushed a commit that referenced this pull request Oct 17, 2025

Add labels to sys.server (#18547)

c19faee

	private Map<String, String> labels;
	private final Map<String, String> labels;

Add labels to sys.server #18547

Add labels to sys.server #18547

Conversation

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

Uh oh!

Uh oh!

FrankChen021 commented Sep 19, 2025

Uh oh!

GabrielCWT commented Sep 19, 2025

Uh oh!

kfaraz commented Sep 19, 2025

Uh oh!

GabrielCWT commented Sep 19, 2025

Uh oh!

Uh oh!

Uh oh!

kfaraz commented Sep 22, 2025

Uh oh!

GabrielCWT commented Sep 23, 2025

Uh oh!

FrankChen021 commented Sep 23, 2025

Uh oh!

kfaraz commented Sep 23, 2025

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!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GabrielCWT Sep 24, 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!

kfaraz Sep 24, 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!

GabrielCWT Sep 24, 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!

kfaraz Sep 24, 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!

kfaraz commented Sep 24, 2025

Uh oh!

FrankChen021 left a comment

Choose a reason for hiding this comment

Uh oh!

kfaraz left a comment

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!

Uh oh!

Uh oh!

FrankChen021 commented Oct 9, 2025

Uh oh!

GabrielCWT commented Sep 19, 2025 •

edited

Loading

GabrielCWT Sep 24, 2025 •

edited

Loading

kfaraz Sep 24, 2025 •

edited

Loading

GabrielCWT Sep 24, 2025 •

edited

Loading

kfaraz Sep 24, 2025 •

edited

Loading