jaegertracing / jaeger-client-csharp

🛑 This library is DEPRECATED!
https://jaegertracing.io/
Apache License 2.0
302 stars 67 forks source link

Configuration.FromIConfiguration gives a NoopSender but should give UdpSender #196

Closed elonmallin closed 3 years ago

elonmallin commented 3 years ago

Requirement

We're using Jaeger and set it up with Configuration.FromIConfiguration which worked in v0.3.6 but updating to v0.4.2 we just get the NoopSender. Is it not possible anymore to use it with the below config to get the UdpSender?

{
"Jaeger": {
    "JAEGER_SERVICE_NAME": "MyServiceName",
    "JAEGER_AGENT_HOST": "192.168.0.1",
    "JAEGER_AGENT_PORT": "6831",
    "JAEGER_SAMPLER_TYPE": "const",
    "JAEGER_SAMPLER_PARAM": "1"
  }
}

According to this old issue it seems like it should be possible: https://github.com/jaegertracing/jaeger-client-csharp/issues/116#issuecomment-510174145

Only downside, the default RemoteReporter would then fallback to a NoopSender if no sender was manually added or if not the Configuration class was used.

We use the Configuration class so thought it would work? I'm not really finding any info on this except explicitly setting the sender which we don't really want since the Configuration.FromIConfiguration gave us all the goodies of considering the configuration we pass in for free. We're using .NET Core (3.1) which the comment mentions has this problem.

Problem

The Configuration.FromIConfiguration seems to just give us a NoopSender even when the config we pass in is supposed to setup the UdpSender.

Falco20019 commented 3 years ago

@elonmallin Please add the following line before calling Configuration.FromIConfiguration:

Configuration.SenderConfiguration.DefaultSenderResolver = new SenderResolver(loggerFactory)
    .RegisterSenderFactory<ThriftSenderFactory>();

On more information about why this was changed, have a look at the main README or https://github.com/jaegertracing/jaeger-client-csharp/blob/master/src/Jaeger.Core/Senders/README.md#notice.

elonmallin commented 3 years ago

Thanks @Falco20019 that works! Guess I missed that page when searching the repo for NoopSender, was trying to dig a bit in the code instead xP Cheers