Problematic tracer metrics defaults

[ahayworth]

👋

Hello, LightStep friends -

It seems that #249 turned metrics reporting on globally, and also bypassed any collector hosts that were configured.

This is causing something of a headache at GitHub, because now our go services are trying to report metrics directly to lightstep-managed ingest points, rather than our own endpoints. Worse still; they're often failing to connect and are now spamming our internal error tracking system.

Was it intentional to enable this by default, and to specifically bypass configured collectors? It seems like this was brought up in the PR review but was not addressed before it was merged.

If possible, could you please push a new version that:

• does not enable the feature by default
• respects the globally-configured collector host, if enabled

We'd appreciate it if you could do that. It'd save us quite a bit of work internally, re-configuring and re-deploying everything.

Some things to consider when deciding if this should be enabled or not, and what hosts should be used:

• We don't want to report metrics outside of known and vetted infrastructure. That poses a security and compliance risk for us, so now this is something of an emergency on my end.
• My team prefers to support one standard metrics configuration internally, and this being enabled by default raises the possibility that teams might begin to use it without our approval and/or knowledge - and that means it's an increased workload on our team that we didn't sign up for and makes our own policy-setting somewhat pointless.
• We specifically configured internal collector pools to absorb the tracing load for our services, and because ingest.lightstep.com isn't keeping up, it's now causing a knock-on problem in our exception tracking system every time the metrics reporter fails to report and raises an exception.
• Even if we did want to use this feature, we haven't necessarily sized our collector pool to handle even more load - so defaulting to the global collector host is not sufficient. We should be able to opt-in, not opt-out.

Thanks!

cc @marwan-at-work

[andrewhsu]

From the conversations outside of this issue, capturing discussion here: we'll address issue by adding a check for env variable upon startup to disable metrics reporting. The reason for this is that the environment variable would be ignored by older releases and you can set it for any future releases, and would require only a redeploy rather than in-code changes.

This would also allow other users of the tracer to continue functioning as they are.

[ahayworth]

Thank you! ❤️ ❤️ ❤️ ❤️

[codeboten]

This change how now been released with v0.21.0