Skip to content

Add Metrics and pusher to unstable #475

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Hmnt39 merged 19 commits into from
Sep 23, 2025
Merged

Add Metrics and pusher to unstable #475

Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
71e16f6
Add Metrics and pusher to unstable
Hmnt39 Sep 5, 2025
1eef3d4
Call push manager only when metrics_config is available
Hmnt39 Sep 5, 2025
eefac40
Add test cases for the metrics
Hmnt39 Sep 8, 2025
a537180
Merge branch 'master' of github.com:cognitedata/python-extractor-util…
Hmnt39 Sep 8, 2025
01cc9d0
Fix the test discovery issue
Hmnt39 Sep 8, 2025
d30f494
Fix the tests
Hmnt39 Sep 8, 2025
c0bd7fd
Refactor changes
Hmnt39 Sep 9, 2025
c22cf01
Reuse the stable metrics
Hmnt39 Sep 9, 2025
37b537e
Merge branch 'master' of github.com:cognitedata/python-extractor-util…
Hmnt39 Sep 10, 2025
1c71ae4
Merge branch 'master' of github.com:cognitedata/python-extractor-util…
Hmnt39 Sep 15, 2025
278b8b0
Add test case for config values
Hmnt39 Sep 15, 2025
9bbc5e8
Merge branch 'master' into DOG-5996-Implement-metrics
Hmnt39 Sep 15, 2025
5101df9
Use yaml for config: PR review
Hmnt39 Sep 17, 2025
6d6141a
Minor Fixes
Hmnt39 Sep 17, 2025
fdfa763
Merge branch 'master' into DOG-5996-Implement-metrics
Hmnt39 Sep 17, 2025
aa34f49
Merge branch 'master' into DOG-5996-Implement-metrics
Hmnt39 Sep 19, 2025
7575f1d
Add test for the COgniteMetrics Config
Hmnt39 Sep 19, 2025
c931f37
Merge branch 'master' into DOG-5996-Implement-metrics
Hmnt39 Sep 22, 2025
ac2a117
Merge branch 'master' into DOG-5996-Implement-metrics
Hmnt39 Sep 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 19 additions & 23 deletions tests/test_unstable/test_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,16 @@
import logging
import random
from collections.abc import Generator
from io import StringIO
from pathlib import Path

import pytest
import yaml

from cognite.client import CogniteClient
from cognite.client.exceptions import CogniteNotFoundError
from cognite.extractorutils.metrics import PrometheusPusher
from cognite.extractorutils.statestore.watermark import LocalStateStore, RawStateStore
from cognite.extractorutils.unstable.configuration.loaders import load_file
from cognite.extractorutils.unstable.configuration.loaders import ConfigFormat, load_io
from cognite.extractorutils.unstable.configuration.models import (
ConnectionConfig,
LocalStateStoreConfig,
Expand Down Expand Up @@ -298,30 +298,26 @@ def counting_push(self: "PrometheusPusher") -> None:
assert call_count["count"] > 0


def test_pushgatewayconfig_none_credentials_from_yaml(tmp_path: Path) -> None:
yaml_config = {
"push_gateways": [
{
"host": "http://localhost:9091",
"job_name": "test-job",
"clear_after": None,
}
],
"cognite": None,
"server": None,
}
yaml_file = tmp_path / "metrics_config.yaml"
with open(yaml_file, "w") as f:
yaml.dump(yaml_config, f)
def test_pushgatewayconfig_none_credentials_from_yaml() -> None:
config_str = """
push-gateways:
- host: "http://localhost:9091"
job_name: "test-job"
clear_after: null
username: null
cognite: null
server: null
Copy link
Copy Markdown
Contributor

@einarmo einarmo Sep 17, 2025

Choose a reason for hiding this comment

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

The thing I wanted to verify was that this works without explicitly setting x: null, so leave those out. We generally don't encourage explicitly setting things to null in yaml configs.

Copy link
Copy Markdown
Contributor Author

@Hmnt39 Hmnt39 Sep 17, 2025

Choose a reason for hiding this comment

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

Here, password is not set explicitly to null, so this is working (I want to verify if explicitly setting to null is also working i.e. username).

Copy link
Copy Markdown
Contributor

@einarmo einarmo Sep 17, 2025

Choose a reason for hiding this comment

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

Then add another test where you don't set them to null explicitly. Setting them to null is the exception, not the rule, and I would prefer if tests verified the intended usage as well as any corner cases.

Copy link
Copy Markdown
Contributor Author

@Hmnt39 Hmnt39 Sep 17, 2025

Choose a reason for hiding this comment

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

The thing I wanted to verify was that this works without explicitly setting x: null, so leave those out. We generally don't encourage explicitly setting things to null in yaml configs.

Made the changes, I got your point why we do require to set the defaults and not passing null explicitly.


metrics_config = load_file(yaml_file, MetricsConfig)
"""

config = metrics_config.push_gateways[0]
stream = StringIO(config_str)
config = load_io(stream, ConfigFormat.YAML, MetricsConfig)
metrics_config = config.push_gateways[0]
pusher = PrometheusPusher(
job_name=config.job_name,
username=config.username,
password=config.password,
url=config.host,
job_name=metrics_config.job_name,
username=metrics_config.username,
password=metrics_config.password,
url=metrics_config.host,
push_interval=30,
thread_name="TestPusher",
cancellation_token=None,
Expand Down
Loading