Skip to content
This repository has been archived by the owner on Oct 3, 2023. It is now read-only.

add golang bazel build support #132

Merged
merged 1 commit into from
Oct 9, 2018

Conversation

yancl
Copy link
Contributor

@yancl yancl commented Oct 8, 2018

No description provided.

@yancl
Copy link
Contributor Author

yancl commented Oct 8, 2018

@songy23
PTAL when you have time, thanks.

@songy23 songy23 requested review from songy23 and bogdandrutu and removed request for songy23 October 8, 2018 15:52
@songy23
Copy link
Contributor

songy23 commented Oct 8, 2018

Why do we need Go proto build, given that we already have the gen-go files?

@songy23 songy23 added the build label Oct 8, 2018
@songy23 songy23 requested a review from rakyll October 8, 2018 16:32
@rakyll
Copy link
Contributor

rakyll commented Oct 8, 2018

We don't need this. bazel don't generate the final artifacts, that's why we use the protoc directly to generate those files.

@bogdandrutu
Copy link
Contributor

@yancl this is very nice. can you please give me some context why do you need this? Do you use bazel on your project and want to build using bazel everything?

@yancl
Copy link
Contributor Author

yancl commented Oct 9, 2018

Thank you all for your review:)

@bogdandrutu yes, we almost build everything using bazel.

The problem is that we want to marshal the spans and send it to kafka through the ocagent. Then some jobs can consume the spans and make some analysis.

But as you see, there isn't some proto already defined in the repo like the following that support this, so we need to define some proto that references the upstream trace proto and support Bazel build. So we think maybe the PR will be useful.

syntax = "proto3";

package dump;

import "opencensus/proto/trace/v1/trace.proto";

// Used to dump spans for later store&process
message DumpSpans {
  repeated opencensus.proto.trace.v1.Span spans = 1;
}

BTW, do you think give a PR like DumpSpans above is a good idea? Thanks.

@bogdandrutu
Copy link
Contributor

I think the "DumpSpan" should not live in this repo. Everything else looks great.

@bogdandrutu bogdandrutu merged commit 475e1d0 into census-instrumentation:master Oct 9, 2018
@odeke-em
Copy link
Member

odeke-em commented Oct 9, 2018

The problem is that we want to marshal the spans and send it to kafka through the ocagent. Then some jobs can consume the spans and make some analysis.

@yancl please take a look at the already published https://godoc.org/github.com/census-instrumentation/opencensus-proto/gen-go/trace/v1 but also we provide functionality for uploading spans from OpenCensus-Go instrumented applications to ocagent in https://godoc.org/contrib.go.opencensus.io/exporter/ocagent#NewExporter

@yancl
Copy link
Contributor Author

yancl commented Oct 10, 2018

@odeke-em thank you for your great information. Now we almost use the same mechanism you point, opencensus-go instrumented applications in each container export spans to the ocagent(https://github.com/census-instrumentation/opencensus-service) in the node through grpc, which then aggregates the spans and send them to kafka brokers.

Then some jaeger collectors consume the spans from kafka and write them to jaeger backend in one way, some spark jobs consume the kafka for some extra analysis to fit our own needs in another way.

So that's why we need to dump spans by pb format(support different kind of consumer of different languages).

I found there are zipkin,jaeger,stackdriver and general collector exporter support in the opencensus-service, but no kafka exporter support. So maybe we could add a kafka support in the opencensus-service, is that suitable?

Thanks.

@odeke-em
Copy link
Member

@odeke-em thank you for your great information. Now we almost use the same mechanism you point, opencensus-go instrumented applications in each container export spans to the ocagent(https://github.com/census-instrumentation/opencensus-service) in the node through grpc, which then aggregates the spans and send them to kafka brokers.

Cool, and that's what the opencensus-service is meant to support :)

I found there are zipkin,jaeger,stackdriver and general collector exporter support in the opencensus-service, but no kafka exporter support. So maybe we could add a kafka support in the opencensus-service, is that suitable?

For starters, if you've got the time commitment and can work on stuff, please publish the Kafka exporter and once it is up, we can talk about perhaps integrating in opencensus-service. Adding an exporter to opencensus-service is trivial :) How does that sound? If your Kafka exporter isn't open sourced and if you'd like to keep it closed source, unfortunately we currently don't have a method for dynamically loading external exporters but one can be made e.g. a "discard channel" exporter that just pumps out spans and allows for scraping from external providers.

Great to hear that we already have a user that is seriously consuming the opencensus-service #moreMotivation

@yancl
Copy link
Contributor Author

yancl commented Oct 10, 2018

Glad to hear that, we will open source the kafka exporter when it works fine.

Thanks again for your great work!!!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants