Skip to content

Conversation

@cmd-ntrf
Copy link
Contributor

@cmd-ntrf cmd-ntrf commented Oct 2, 2025

When applying a function to a time series in a query to Prometheus, the Prometheus server returns a data structure without __name__ key. Apart from that aspect, the time series could still be used to create a Metric object.

This PR proposes to set "metric_name" to None when __name__ is missing.

When applying a function to a time series in a query to Prometheus,
the Prometheus server returns a data structure without __name__ key.
Apart from that aspect, the time series could still be used to create
a Metric object.

This PR proposes to set "metric_name" to None when __name__ is missing.
@cmd-ntrf cmd-ntrf changed the title Allow creation of Metric from datastructure without __name__ Allow creation of Metric from data structure without __name__ Oct 2, 2025
@4n4nd 4n4nd requested a review from Copilot December 4, 2025 03:16
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR modifies the Metric class initialization to support creating Metric objects from Prometheus query results that lack a __name__ key, which commonly occurs when applying functions to time series. The change sets metric_name to None when __name__ is absent, rather than raising a KeyError.

Key Changes

  • Modified line 57 to use pop("__name__", None) instead of direct dictionary access
  • Removed the explicit deletion of __name__ from label_config on the previously existing line (now removed), relying on pop() to handle removal

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 57 to 58
self.metric_name = metric["metric"].pop("__name__", None)
self.label_config = deepcopy(metric["metric"])
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

The PR allows metric_name to be None, but there's no test coverage for this new behavior. Consider adding a test case that verifies:

  1. A Metric can be created from data without a __name__ key
  2. The metric_name attribute is correctly set to None
  3. The label_config contains all other labels
  4. Operations like __eq__ and __str__ work correctly with None metric_name

Example test:

def test_metric_without_name(self):
    """Test creating a Metric from data without __name__ key."""
    test_metric = Metric(
        metric={
            "metric": {"instance": "localhost", "job": "test"},
            "value": [1627485628.789, "26.82"],
        }
    )
    self.assertIsNone(test_metric.metric_name)
    self.assertEqual(test_metric.label_config, {"instance": "localhost", "job": "test"})

Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@4n4nd 4n4nd merged commit 5d3ca01 into 4n4nd:master Dec 4, 2025
7 checks passed
@4n4nd
Copy link
Owner

4n4nd commented Dec 4, 2025

@copilot open a new pull request to apply changes based on the comments in this thread

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants