Decouple exporters from Flask integration

[reyang]

This is part of the effort for #394.

1. Introduced a generic configuration evaluation class.
2. Decouple Flask integration and extension exporters.
3. Use W3C TraceContext as the default propagator. Following https://github.com/census-instrumentation/opencensus-specs/blob/master/trace/HTTP.md#propagation

If user doesn't set any propagation methods explicitly, TraceContext is used.

@c24 @songy23 please take a brief look and see if you're okay with the direction.
Once I got okay from you on the direction, I'll work on Django and Pyramid as well.

[reyang]

@c24t @songy23 I wish to know what's the long term configuration structure in your mind.

For example, today we put "PROPAGATOR" under "OPENCENSUS_TRACE", which means if we have tags propagation (e.g. we could have W3C correlation context protocol for tags propagation under HTTP, or a binary protocol for gRPC), either we need to introduce something like OPENCENSUS_TAGS.PROPAGATOR, or decouple propagator from traces (e.g. having a top level propagator concept, which takes the entire context, pick whatever is needed, and serialize/deserialize).
Similar question for log sampler, potentially we will need to have OPENCENSUS_LOG.SAMPLER.

[reyang]

Chatted with @songy23 offline, it seems gRPC integration is taking the approach of separated propagators.

So, the configuration might look like:

app.config['OPENCENSUS'] = {
    'TRACE': {
        'SAMPLER': 'opencensus.trace.samplers.ProbabilitySampler(rate=1)',
        'EXPORTER': 'opencensus.ext.stackdriver.trace_exporter.StackdriverExporter()',
        'PROPAGATOR': 'opencensus.trace.propagation.google_cloud_format.GoogleCloudFormatPropagator()'
    },
    'METRIC':  {
        'EXPORTER': 'opencensus.ext.stackdriver.metrics_exporter.StackdriverExporter()'
    },
    'LOG': {
        'SAMPLER': 'opencensus.trace.samplers.ProbabilitySampler(rate=1)',
        'EXPORTER': 'opencensus.ext.stackdriver.log_exporter.StackdriverExporter()'
    },
    'TAG': {
        'PROPAGATOR': 'opencensus.tags.propagation.CorrelationContextPropagator()'
    }
}

@songy23 please correct me if I'm wrong.

[reyang]

@c24t this also brings the question of where should samplers go, do we want to have ProbabilitySampler under trace or put it in the general place so both traces and logs can share them. Or should log respect the sampling decision if there is a trace context (and what if the customer is using log feature only, without using any of the trace features)?

[c24t]

As I imagine it, the benefit of using samplers with logging is to turn up the log verbosity for requests whose traces are sampled. I don't see a use case for sampling logs without tracing, and sampling seems fundamentally like a feature of tracing.

This gets complicated with tail-based sampling since it means we'd need to send all logs to the agent/collector to be exported or dropped at the end of the request when the sampling decision gets made.

[c24t]

The direction looks great, and this is a long overdue improvement for integrations.

Making config executable code may open up a can of worms, especially since some modules have import time effects... including reading config. I don't see any specific security risks here (since I wouldn't expect untrusted code to be used as config), but we should consider this before release.

This PR changes a lot of defaults, it'd be helpful to list these somewhere before the release. Users might reasonably expect "JaegerExporter" (old) and "opencensus.ext.jaeger.trace_exporter.JaegerExporter()" (new) to produce the same exporters in code.

[c24t]

As I imagine it, the benefit of using samplers with logging is to turn up the log verbosity for requests whose traces are sampled. I don't see a use case for sampling logs without tracing, and sampling seems fundamentally like a feature of tracing.

This gets complicated with tail-based sampling since it means we'd need to send all logs to the agent/collector to be exported or dropped at the end of the request when the sampling decision gets made.

[c24t]

Some of these don't match the default args in the exporter constructor (or they're not set there). I don't see a good way around it, but it'll be surprising for e.g. service_name to change for users using the default jaeger config.

[c24t]

So the goal is to remove OPENCENSUS_TRACE_PARAMS from all integrations, move exporter-specific defaults into the exporters, and push options like BLACKLIST_PATHS up into the app config?

[reyang]

Yep.
And probably instead of having OPENCENSUS_TRACE, use OPENCENSUS as the configuration name (and having the second level TRACE, METRICS, etc.).

[c24t]

Glad we can lose this!

[reyang]

Making config executable code may open up a can of worms, especially since some modules have import time effects... including reading config. I don't see any specific security risks here (since I wouldn't expect untrusted code to be used as config), but we should consider this before release.

I thought about this before making the change. It looks like Python by convention uses *.py file for configuration, which implies evaluation. I plan to fix all TBD docs after the refactor for Django/Pyramid. I will add a note in the document on the potential security consideration.

This PR changes a lot of defaults, it'd be helpful to list these somewhere before the release. Users might reasonably expect "JaegerExporter" (old) and "opencensus.ext.jaeger.trace_exporter.JaegerExporter()" (new) to produce the same exporters in code.

Agree. I will write a section in the release note.