-
Couldn't load subscription status.
- Fork 14
[OpenTelemetry] Postgres metrics updates #5532
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.
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
base: main
Are you sure you want to change the base?
Conversation
|
|
…mat is different than the normal metrics api.
…igure out replica db queries since those metrics require another instance.
|
|
||
| class OtelCollectorScenario(DockerScenario): | ||
| def __init__(self, name: str): | ||
| use_proxy = os.environ.get("OTELCOLLECTOR_PROXY", "true").lower() == "true" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A given scenario can't use both mode, it would make test inconsistent.
Rather, you must declare two scenario, one that mocks the backend (and so do not perform any backend based assertions), and another one that do not mock it, and can perform backend-based assertions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback! I'll take a look and see how splitting affects what I'm trying to do.
| scenario_groups=[scenario_groups.end_to_end], | ||
| include_postgres_db=True, | ||
| use_proxy=True, | ||
| use_proxy=use_proxy, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just keep the proxy, having the logs is very helpful, and does not prevent using the real backend.
| ) | ||
| self.library = ComponentVersion("otel_collector", "0.0.0") | ||
|
|
||
| collector_env: dict[str, str | None] = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part must be moved inside OpenTelemetryCollectorContainer constructor.
you can also move the _require_api_key from EndToEndScenario to DockerScenario to add a safe guard
| } | ||
| ) | ||
|
|
||
| if use_proxy: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use case is that the Otel Collector Postgres yaml file would have access to the environment variable to publish as a metric tag. Is there another way to grab this information already?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The point is you'll use the proxy in both mode (real backend, or mocked backend). So you will never have use_proxy==False.
Co-authored-by: Charles de Beauchesne <[email protected]>
…onment variable for OTELCOLLECTOR_PROXY
Motivation
For AIDM-147, the goal is to have a way to manually send the data from the test to a non proxy backend.
Instructions:
For a specific test:
For all tests
Changes
Workflow
🚀 Once your PR is reviewed and the CI green, you can merge it!
🛟 #apm-shared-testing 🛟
Reviewer checklist
[<language>], double-check that only<language>is impacted by the changebuild-XXX-imagelabel is present