open-telemetry / opentelemetry-specification

Specifications for OpenTelemetry
https://opentelemetry.io
Apache License 2.0
3.69k stars 884 forks source link

Add w3c TraceContext propagation integration tests #749

Open srjames90 opened 4 years ago

srjames90 commented 4 years ago

What are you trying to achieve? W3c has an external validator to verify that the TraceContext spec is adhered to https://github.com/w3c/trace-context/tree/master/test. Opentelemtry-python has already integrated these tests into their own repo (https://github.com/open-telemetry/opentelemetry-python/issues/202), and I think it'd be good to do something like this in all of the languages.

Additional context. These are the relevant files in the opentelemetry-python repo: https://github.com/open-telemetry/opentelemetry-python/blob/master/tests/w3c_tracecontext_validation_server.py https://github.com/open-telemetry/opentelemetry-python/blob/master/scripts/tracecontext-integration-test.sh

I've already written similar validation servers to the python server but in Go, Java, and JS. They work with a modified version of the shell script above, just need to figure out how to integrate them into the different repos.

tigrannajaryan commented 3 years ago

This does not seem to be a specification issue. It is about adding integration tests to SDKs.

There is an open PR which attempts to add specification for tests: https://github.com/open-telemetry/opentelemetry-specification/pull/630

If this issue is indeed about coming up with a common specification about how the tests should be implemented (and not an issue to actually implement the tests) then I think the description needs to be clarified.

Either way I do not think this is a must have to the GA (although highly desirable to have good test coverage). I suggest to remove release:required-for-ga label.

andrewhsu commented 3 years ago

From the issue triage mtg today, i'm changing the label to release:after-ga since it looks like from the comments this can be punted.