Skip to content

Add key_property support for dynamic_tags#601

Merged
gh-worker-dd-mergequeue-cf854d[bot] merged 2 commits into
masterfrom
piotr.wolski/dynamic-tags-key-property
Mar 30, 2026
Merged

Add key_property support for dynamic_tags#601
gh-worker-dd-mergequeue-cf854d[bot] merged 2 commits into
masterfrom
piotr.wolski/dynamic-tags-key-property

Conversation

@piochelepiotr

Copy link
Copy Markdown
Collaborator

Summary

  • Extends dynamic_tags to support extracting tag values from JMX bean name key properties
  • New key_property field as an alternative to attribute — uses ObjectName.getKeyProperty() to extract the value
  • Bean name patterns with wildcards (*) are supported via queryNames()

Motivation

Some important values only exist as key properties in JMX bean names, not as attribute values. For example, Kafka's broker ID is only available in beans like kafka.server:type=app-info,id=1 — there is no attribute that returns the broker ID.

This makes it possible to write:

dynamic_tags:
  - tag_name: broker_id
    bean_name: kafka.server:type=app-info,id=*
    key_property: id

Test plan

  • New unit test: testConfigDynamicTagsKeyProperty — verifies key property extraction from bean name
  • New unit test: testConfigDynamicTagsKeyPropertyAndAttribute — verifies key_property and attribute dynamic tags work together on the same metric

🤖 Generated with Claude Code

Extend dynamic_tags to extract tag values from JMX bean name key
properties, not just attribute values. This enables tagging metrics
with values like Kafka's broker ID which only exists as a key property
in bean names (e.g. kafka.server:type=app-info,id=1).

New syntax:
  dynamic_tags:
    - tag_name: broker_id
      bean_name: kafka.server:type=app-info,id=*
      key_property: id

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@piochelepiotr piochelepiotr requested a review from a team as a code owner March 23, 2026 16:42
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@carlosroman

Copy link
Copy Markdown
Collaborator

@codex review

}

@Test
public void testConfigDynamicTagsKeyProperty() throws Exception {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you for adding these tests ❤️

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b7df6615fc

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +148 to +149
ObjectName matchedBean = matchingBeans.iterator().next();
String value = matchedBean.getKeyProperty(keyProperty);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Handle multiple key-property matches deterministically

The new key_property resolver takes the first element from queryNames() via matchingBeans.iterator().next(), but wildcard bean_name patterns can legally match multiple MBeans and Set iteration order is not stable. In those cases, the chosen bean (and emitted dynamic tag value) becomes nondeterministic across runs, which can mis-tag metrics and corrupt aggregations. Please enforce a single-match contract (or deterministic selection with explicit tie-breaking) before extracting the key property.

Useful? React with 👍 / 👎.

@carlosroman carlosroman self-requested a review March 30, 2026 09:08
@carlosroman

Copy link
Copy Markdown
Collaborator

/merge

@gh-worker-devflow-routing-ef8351

gh-worker-devflow-routing-ef8351 Bot commented Mar 30, 2026

Copy link
Copy Markdown

View all feedbacks in Devflow UI.

2026-03-30 09:10:05 UTC ℹ️ Start processing command /merge


2026-03-30 09:10:10 UTC ℹ️ MergeQueue: pull request added to the queue

The expected merge time in master is approximately 14m (p90).


2026-03-30 09:23:08 UTC ℹ️ MergeQueue: This merge request was merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants