chore(zipkin): adds span serializer for different formats. #3
Conversation
Signed-off-by: José Carlos Chávez <[email protected]>
| return stringified_json_array; | ||
| } | ||
|
|
||
| std::string SpanBuffer::toStringifiedJsonArray(SpanSerializer& span_serialier) { |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
jcchavezs
changed the title
| stringified_json_array += "]"; | ||
|
|
||
| return stringified_json_array; | ||
| std::string SpanBuffer::serialize(SpanSerializer& serializer) { |
There was a problem hiding this comment.
I was always curious about a good example for visitor pattern. I think this one is a good one.
| */ | ||
| std::string toStringifiedJsonArray(); | ||
| std::string serialize(SpanSerializer& serializer) |
There was a problem hiding this comment.
Now SpanBuffer does not have to be aware of proto or json serialization.
…ffers. Signed-off-by: José Carlos Chávez <[email protected]>
jcchavezs
force-pushed
the
adds_span_serializer
branch
from
a603178 to
e77f39e
Compare
| @@ -15,31 +15,33 @@ TEST(ZipkinSpanBufferTest, defaultConstructorEndToEnd) { | |||
| SpanBuffer buffer(envoy::config::trace::v2::ZipkinConfig::HTTP_JSON_V1); | |||
|
|
|||
| EXPECT_EQ(0ULL, buffer.pendingSpans()); | |||
| EXPECT_EQ("[]", buffer.toStringifiedJsonArray()); | |||
| // tests are commented out for now, I will move them to serialization once | |||
| // have more expertise with tests. | |||
| @@ -126,18 +126,75 @@ Tracing::SpanPtr Driver::startSpan(const Tracing::Config& config, Http::HeaderMa | |||
| return active_span; | |||
| } | |||
|
|
|||
| HTTPSpanSerializerV1::HTTPSpanSerializerV1() {} | |||
There was a problem hiding this comment.
@dio is it OK if I move this into another file? I guess yes (as long as that file is part of the compilation like in C) but I want to double check.
| */ | ||
| virtual std::string serialize(std::vector<Span> spans); | ||
|
|
||
| virtual std::string contentType(); |
There was a problem hiding this comment.
The serializer is the one that knows the content type so it makes sense we get that information from here.
jcchavezs
changed the title
|
OK, this is interesting. I have pushed the "base" PR for us to work on (I'll fix the build problem if that exists). |
|
This is the corresponding branch in this repo: https://github.com/envoy-zipkin/envoy/tree/zipkin-proto3 |
Integrating span serializer.
In a next PR I will move all the serialization from the span into the serializators.
Ping @adriancole @dio