Skip to content

Use the Zipkin library to support Zipkin tracing#700

Closed
fabolive wants to merge 9 commits into
envoyproxy:masterfrom
amalgam8:use-zipkin-library
Closed

Use the Zipkin library to support Zipkin tracing#700
fabolive wants to merge 9 commits into
envoyproxy:masterfrom
amalgam8:use-zipkin-library

Conversation

@fabolive
Copy link
Copy Markdown
Contributor

@fabolive fabolive commented Apr 5, 2017

Enables Zipkin tracing. This PR makes Envoy use the code added by PR #692.

cc: @RomanDzhabarov, @adriancole, @mattklein123, @rshriram

@fabolive fabolive mentioned this pull request Apr 5, 2017
"access_token_file" : {"type" : "string"},
"endpoint": {"type": "string"}
},
"required": ["collector_cluster", "access_token_file"],
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be done in a way that there are separate schemas for different driver types, otherwise schema will not have much value.

#include "common/runtime/uuid_util.h"

#include "zipkin/zipkin_core_constants.h"
#include <iostream>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this

}

void ZipkinReporter::onFailure(Http::AsyncClient::FailureReason) {
std::cerr << "Error posting spans to Zipkin" << std::endl;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for things like that we should have stats.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep.. its coming in the next commit..

@rshriram rshriram mentioned this pull request Apr 8, 2017

void ZipkinReporter::onFailure(Http::AsyncClient::FailureReason) {
std::cerr << "Error posting spans to Zipkin" << std::endl;
// TODO: stats
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in case it helps, here's an example of another tracer's stats:

https://github.com/openzipkin/zipkin-finagle#metrics

most use cases are comparing against server stats:

https://github.com/openzipkin/zipkin/blob/master/zipkin-server/README.md#collector

@mattklein123
Copy link
Copy Markdown
Member

Closing this for now until we get the other PR merged.

htuch pushed a commit that referenced this pull request May 3, 2017
This PR adds a Zipkin library to the Envoy code base. Changes made to Envoy to use this library to support Zipkin tracing are in PR #700.
@fabolive fabolive changed the title [WIP] Use the Zipkin library to support Zipkin tracing Use the Zipkin library to support Zipkin tracing May 5, 2017
@fabolive fabolive mentioned this pull request May 5, 2017
rshriram pushed a commit to rshriram/envoy that referenced this pull request Oct 30, 2018
mathetake added a commit that referenced this pull request Mar 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants