Build OpenTelemetry.Exporter.OpenTelemetryProtocol.csproj based on latest .proto files

[pcwiese]

.proto files forked from https://github.com/open-telemetry/opentelemetry-proto
@4b6ff88e525739a2b1d2cbed619301076c6687e7

Generate the .cs files from the .proto files using Grpc.Tools.

Related:
#19

[SergeyKanzhelev]

I didn't review much, but looks good after the first glance. Do you want it merged right away?

[SergeyKanzhelev]

perhaps add a file with the SHA of a copy in the repository

[pcwiese]

I didn't review much, but looks good after the first glance. Do you want it merged right away?

Certainly not urgent. Just trying to get something flowing from C# to the otelcol container for testing.

After looking more carefully at the collector...it isn't even referencing these new .proto files at all, so there is really no rush. There is nothing capable of hosting a service that this exporter can talk to yet I think. Maybe I'm missing something.

[SergeyKanzhelev]

@pcwiese no, you are correct. I believe the work is planned though. I saw you following up on gitter on this.

Let's keep it in PR for a bit and once there is a timeline we can move it forward

[SergeyKanzhelev]

Actually, let's merge this in as we already have project merged. Do you mind renaming the project to OTCollector?

[pcwiese]

Sure I can rename it

…

Sent from my iPhone

On Nov 7, 2019, at 15:53, Sergey Kanzhelev ***@***.***> wrote:  Actually, let's merge this in as we already have project merged. Do you mind renaming the project to OTCollector? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[lmolkova]

Some comments:

• i don't think current protos follow approved OTEP: https://github.com/open-telemetry/oteps/blob/master/text/0035-opentelemetry-protocol.md. E.g. look into resource span or trace request/response. Is this the right time to return this exporter back?

• naming:

  • is there a decision to name it 'collector' across OTel? E.g. in Java it's called otprotocol. Collector term is very misleading (normally something that collects telemetry, not exports it) and if it's not a spec decision, we should call it something else.

  • I believe it's a standard convention to avoid consequent capital letters. I'd prefer OtCollector or Otcollector instead of OTCollector

There are few extra things we need to do for this exporter (probably as extra PRs):

• add extendable options class instead of public TraceExporter(string agentEndpoint, string hostName, string serviceName, ChannelCredentials credentials = null, uint spanBatchSize = MaxSpanBatchSize)
• Add UseOtCollector extensions methods (similar to Zipkin) on TracerFactoryBuilder to help with registration and update README.

[pcwiese]

I 100% agree with all of your comments here.

I'm aware the current .protos aren't spec approved compliant and I had no plans to update this until the actual collector receiver was in place anyway. So it might be too early. I added it back because I inadvertently thought the collector had some otel protocol receiver available; but there is none at all right now.

The exporter itself needs a bunch of work, agreed.
I intentionally did not update the README because people shouldn't be using yet.

I do think there is value in getting this in though. There are plenty of tasks for someone to do apart from the transformation from OpenTelemetry SDK constructs to wire protocol constructs.

Happy to shelve this for now, or just get it in and keep it in sync when the .protos change.

If we want to check this in, lets agree on a name. I happen to like OpenTelemetry.Exports.OtProtocol.

[lmolkova]

@pcwiese I'm fine with merging it as long as we figure out naming.

@SergeyKanzhelev what is the motivation for OTCollector and can we name it differently?
According to the spec, collector is rather daemon or service.

Can we call it

• Otprocotol (like java) - I'm not fun, but its better than collector
• Otlp (this is spec abbreviation for the protocol)
• OtCollector or Otcollector if it has to be called collector?

[pcwiese]

I vote for either Otlp or just OpenTelemetryProtocol (despite being a bit redundant). Usually I am favor of spelling things out to reduce confusion.

[SergeyKanzhelev]

Full name of the package is OpenTelemtery.Exporter.OTCollector similar to OpenTelemtery.Exporter.Jaeger. So it is hard to confuse with the daemon. But I'm fine with OpenTelemetryProtocol.

[pcwiese]

I'll go with OpenTelemetryProtocol then.

[SergeyKanzhelev]

let's handle adding extension method in separate PR. Merging this one