Skip to content
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

Jump to bottom

Wire compatibility tests #1031

Open
tigrannajaryan opened this issue May 26, 2020 · 6 comments
Open

Wire compatibility tests #1031

tigrannajaryan opened this issue May 26, 2020 · 6 comments
Labels
help wanted Good issue for contributors to OpenTelemetry Service to pick up
Milestone
Backlog

Comments

@tigrannajaryan
Copy link
Member

We need compatibility tests that verify that the build is compatible on wire level with previous official release (chain new and old versions and ensure data flows correctly).
@tigrannajaryan tigrannajaryan added feature request help wanted Good issue for contributors to OpenTelemetry Service to pick up and removed feature request labels May 26, 2020
@tigrannajaryan tigrannajaryan mentioned this issue May 28, 2020
Closed
@kbrockhoff kbrockhoff mentioned this issue Jun 1, 2020
Merged
@flands flands added this to the Beta 0.4 milestone Jun 4, 2020
@tigrannajaryan
Copy link
Member Author

@flands I am removing this from this milestone since we won't have time to do it. Will be done later.

@tigrannajaryan tigrannajaryan removed this from the Beta 0.4 milestone Jun 4, 2020
tigrannajaryan pushed a commit that referenced this issue Jun 17, 2020
@kbrockhoff
Generalize testbed and add trace translation correctness tests (#1062)
e3df9f3 
Extracted out TestResultsSummary (in testbed/testbed/results.go), DataProvider (in testbed/testbed/data_provider.go), OtelcolRunner (in testbed/testbed/otelcol_runner.go), TestCaseValidator (in testbed/testbed/validator.go) interfaces with multiple implementations. Added tracing correctness tests in testbed/correctness using the testbed with different implementations of the 5 interfaces listed than what the perf tests use.

**Link to tracking Issue:** Provides the support to cleanly implement #652, #1022, #1023, #1027, #1031

**Testing:** All existing testbed-driven tests still pass. Correctness tests run without any panics. Correctness tests are reporting a number of bugs with translations.

**Documentation:** Godocs on all public methods.
wyTrivail pushed a commit to mxiamxia/opentelemetry-collector that referenced this issue Jul 13, 2020
@kbrockhoff @wyTrivail
Generalize testbed and add trace translation correctness tests (open-…
874ade0 
…telemetry#1062)

Extracted out TestResultsSummary (in testbed/testbed/results.go), DataProvider (in testbed/testbed/data_provider.go), OtelcolRunner (in testbed/testbed/otelcol_runner.go), TestCaseValidator (in testbed/testbed/validator.go) interfaces with multiple implementations. Added tracing correctness tests in testbed/correctness using the testbed with different implementations of the 5 interfaces listed than what the perf tests use.

**Link to tracking Issue:** Provides the support to cleanly implement open-telemetry#652, open-telemetry#1022, open-telemetry#1023, open-telemetry#1027, open-telemetry#1031

**Testing:** All existing testbed-driven tests still pass. Correctness tests run without any panics. Correctness tests are reporting a number of bugs with translations.

**Documentation:** Godocs on all public methods.
@bogdandrutu bogdandrutu added this to the GA 1.0 milestone Aug 4, 2020
@tigrannajaryan tigrannajaryan modified the milestones: GA 1.0, Backlog Oct 2, 2020
MovieStoreGuy pushed a commit to atlassian-forks/opentelemetry-collector that referenced this issue Nov 11, 2021
@Aneurysm9 @MrAlias
Move content length out of basic attributes (open-telemetry#1031)
b40fdf1 
* Move content length out of basic attributes

semconv.httpBasicAttributesFromHTTPRequest() was including the request's content length,
which is a high-cardinality label.  It ended up in metric labels through the use of that function
by semconv.HTTPServerMetricAttributesFromHTTPRequest().

* Add CHANGELOG entry

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
@MovieStoreGuy
Copy link
Contributor

I am happy to pick this up @tigrannajaryan,

Is that we only care about N and N+1 versions? Should that be configurable? And should this be for all receivers / exporters, or specifically scoped to the otlp receivers / exporters?

@tigrannajaryan
Copy link
Member Author

I think verifying N and N+1 is sufficient. The purpose of the test is to catch accidental breakage due to any changes we make in the serialization or transport code (e.g. we are currently looking at replacing some generated Protobuf code by manually written code, which is error prone). I think we want it for OTLP which evolves over time (both protocol spec and the implementation). the rest of the receivers/exporters we rarely touch after the initial implementation.

There are probably many ways a test like this can be written. One possible way is to download the last stable Collector docker image and test that against one built from HEAD. This should catch any incompatibilities early in the PRs. I hope we can reuse OTLP data generators used in the correctness tests.

@MovieStoreGuy
Copy link
Contributor

That is sounds reasonable with me, let me explore some ideas and I'll come back with a PR hopefully in the next week or two?

@tigrannajaryan
Copy link
Member Author

Sounds good.

@atoulme
Copy link
Contributor

atoulme commented Dec 14, 2023

@MovieStoreGuy are you still working on this? Can I assign this ticket to you?

Troels51 pushed a commit to Troels51/opentelemetry-collector that referenced this issue Jul 5, 2024
@esigo
Add Prometheus Exporter (open-telemetry#1031)
994c08a
swiatekm pushed a commit to swiatekm/opentelemetry-collector that referenced this issue Oct 9, 2024
@dhrp
Fixed changed connectors.spanmetrics config, closes open-telemetry#1031…
0799351 
… (open-telemetry#1032)

* changed connectors.spanmetrics config

* bump helm chart version

* Updated the examples using make generate-examples
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
help wanted Good issue for contributors to OpenTelemetry Service to pick up
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
5 participants
@atoulme @flands @bogdandrutu @tigrannajaryan @MovieStoreGuy