-
Notifications
You must be signed in to change notification settings - Fork 438
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
Add a test service for the W3C validation harness #487
Add a test service for the W3C validation harness #487
Conversation
examples/CMakeLists.txt
Outdated
@@ -6,3 +6,4 @@ add_subdirectory(simple) | |||
add_subdirectory(batch) | |||
add_subdirectory(metrics_simple) | |||
add_subdirectory(multithreaded) | |||
add_subdirectory(w3c_tracecontext_test) |
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.
Put w3c_tracecontext_test into some test folder instead of under examples?
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.
Yes, I'll move it. Probably I'll set up something similar to what C# people did: https://github.com/open-telemetry/opentelemetry-dotnet/tree/master/test/OpenTelemetry.Instrumentation.W3cTraceContext.Tests
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.
Yes, docker image or script to run both the services and give result would be handy ( as part of separate PR )
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 is moved now into ext
, signaling that this is not a central component or test that has to run with the core test suite.
Codecov Report
@@ Coverage Diff @@
## master #487 +/- ##
==========================================
+ Coverage 94.46% 94.48% +0.01%
==========================================
Files 189 189
Lines 8388 8388
==========================================
+ Hits 7924 7925 +1
+ Misses 464 463 -1
|
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.
LGTM once CI checks are fixed. . Thanks for this adding validation code.
Most failures are related to tracestate propagation, which we currently do not support at all.
Should we be creating a ticket for this ?
port = std::stoi(uri.substr(7 + host_end + 1, port_end)); | ||
path = uri.substr(host_end + port_end + 2, std::string::npos); | ||
} | ||
}; |
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.
jfi - i added a url_parser.h header to parse the url as part of zipkin exporter PR ( https://github.com/open-telemetry/opentelemetry-cpp/pull/471/files )
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.
I'd love to use that once it's merged.
#include "opentelemetry/trace/scope.h" | ||
|
||
#include <algorithm> | ||
#include "nlohmann/json.hpp" |
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.
Where / how do you install nlohmann/json.hpp
for non-Windows OS?
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 relies on the nlohmann-json
packages installed by our CI tools.
e2c6bd2
to
b20f7e1
Compare
This should be ready for another review and hopefully merge, all the CI tests are passing. |
This example program is intended to be used as a test service for the W3C Distributed Tracing Validation Service. It is implemented according to this instructions.
With the current state of our implementation, 26 out of 40 tests pass. Most failures are related to
tracestate
propagation, which we currently do not support at all.This program makes use of
nlohmann::json
and our internally provided HTTP client and server implementations.