opentelemetry-docker-tests: Refactor Docker tests to properly validate contents of exported telemetry#5220
opentelemetry-docker-tests: Refactor Docker tests to properly validate contents of exported telemetry#5220herin049 wants to merge 28 commits into
Conversation
xrmx
left a comment
xrmx
left a comment
There was a problem hiding this comment.
As a reminder for some reason the stuff we add to test-utils may also be used by downstream users
@xrmx I figured we can maybe keep the server class public since some users might find it useful. Alternatively, I can make it private for now. Let me know what you think. |
Removed PyYAML and requests dependencies from docker-tests environment.
There was a problem hiding this comment.
Pull request overview
Refactors opentelemetry-docker-tests OTLP exporter Docker-based functional tests so they validate the semantic contents of exported telemetry (traces/metrics/logs), by forwarding collector-received OTLP data to an in-process OTLP/HTTP test server for assertions.
Changes:
- Introduces an OTLP protobuf-over-HTTP test server (
OtlpProtoTestServer) plus unit tests for it inopentelemetry-test-utils. - Reworks Docker OTLP exporter functional tests to assert on exported span/metric/log record contents (including adding logs functional coverage).
- Updates docker test orchestration to use the system
dockerCLI withdocker compose, and adds a collector config that forwards telemetry to the in-process test server.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tox.ini | Updates docker-tests env dependencies and switches orchestration to docker compose; reintroduces explicit lint env for test-utils. |
| tests/opentelemetry-test-utils/tests/test_otlp_test_server.py | Adds tests covering the OTLP test server behavior across traces/metrics/logs and compression. |
| tests/opentelemetry-test-utils/test-requirements.txt | Adds OTLP proto HTTP exporter dependency for test-utils test runs. |
| tests/opentelemetry-test-utils/src/opentelemetry/test/otlp_test_server.py | Adds an in-process OTLP protobuf-over-HTTP server that records received telemetry into queues for assertions. |
| tests/opentelemetry-docker-tests/tests/otlpexporter/test_otlp_traces_functional.py | Adds trace functional tests for both OTLP/HTTP and OTLP/gRPC exporters (via shared base). |
| tests/opentelemetry-docker-tests/tests/otlpexporter/test_otlp_metrics_functional.py | Adds metrics functional tests for both OTLP/HTTP and OTLP/gRPC exporters (via shared base). |
| tests/opentelemetry-docker-tests/tests/otlpexporter/test_otlp_logs_functional.py | Adds logs functional tests for both OTLP/HTTP and OTLP/gRPC exporters (via shared base). |
| tests/opentelemetry-docker-tests/tests/otlpexporter/test_otlp_http_exporter_functional.py | Removes prior HTTP exporter tests that only validated receipt rather than semantic correctness. |
| tests/opentelemetry-docker-tests/tests/otlpexporter/test_otlp_grpc_exporter_functional.py | Removes prior gRPC exporter tests that only validated receipt rather than semantic correctness. |
| tests/opentelemetry-docker-tests/tests/otlpexporter/init.py | Replaces prior “received successfully” checks with semantic validation base classes for traces/metrics/logs using the in-process server. |
| tests/opentelemetry-docker-tests/tests/docker-compose.yml | Updates collector service to use an explicit config file and adds host gateway mapping for forwarding. |
| tests/opentelemetry-docker-tests/tests/collector-config.yaml | Adds collector pipelines that forward traces/metrics/logs to the host OTLP/HTTP test server. |
| .changelog/5220.added | Adds changelog entry for the docker-tests refactor. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| recorded = self._server.get_log_record(timeout=5.0) | ||
| attrs = _attrs_to_dict(recorded.log_record.attributes) | ||
| self.assertEqual(attrs["str_key"], "hello") | ||
| self.assertEqual(attrs["int_key"], 42) | ||
| self.assertAlmostEqual(attrs["float_key"], 3.14, places=5) | ||
| self.assertEqual(attrs["bool_key"], True) |
There was a problem hiding this comment.
WDYT of using inline-snapshot for these assertions? IMO it makes adding new tests or updating existing ones if needed very easy.
OTOH the timestamps will need special handling and it might be clunky to deal with. We don't have to do it, just looking for your thoughts to start.
@xrmx for thoughts too
There was a problem hiding this comment.
Sounds good to me, up to @herin049 if we want to give it a run in this PR.
There was a problem hiding this comment.
I've updated the PR to include some limited use of inline-snapshot to give it a try. I think it's useful in situations like these were we are validating the shape/structure of something that is well defined/deterministic. I think we would benefit the most from utilizing this package for some of the tests inside the contrib repo.
That being said, in my opinion we should be careful not to go overboard with this package by creating large snapshots that could result in validating behavior that isn't relevant to the individual test. There is a possibility that overuse can result in our tests becoming more brittle, and as a result we end up running --inline-snapshot=fix frequently, eliminating the value of the test in the first place.
There was a problem hiding this comment.
Thanks could you please add a note in CONTRIBUTING.md saying that docker tests are using inline-snapshots with a link to the repo or doc please?
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
I've decided to keep the test server private for now. |
Description
Prior to this refactor, the existing Docker exporter tests only verify that telemetry is correctly received by the OpenTelemetry collector. However, it does not validate that the contents of the telemetry received by the collector is semantically correct.
This refactor updates the Docker tests to properly validate the correctness of the contents of the telemetry received by the collector by updating the collector to forward all received telemetry to an additional OTLP HTTP server listening in the test runner process. The
pytesttests are then able to examine the forwarded telemetry and verify the correctness of the contents of the telemetry, and that nothing was lost or mangled during exporting.This PR also switches to using the system
dockerexecutable and likewise switches from usingdocker-composetodocker compose. Furthermore, it also adds functional tests for logging, which were previously not present.Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Does This PR Require a Contrib Repo Change?
Checklist: