Add config options from env for BatchSpanProcessor #228
Conversation
| @@ -269,6 +283,56 @@ impl BatchSpanProcessor { | |||
| config: Default::default(), | |||
| } | |||
| } | |||
|
|
|||
| /// Create a new batch processor builder and set the config value based on environment variables. | |||
| pub fn from_env<E, S, SO, I, IO>( | |||
There was a problem hiding this comment.
Do you prefer the explicit from_env here? Has advantages of being clear when it is looking from env, alternative could be to have the batch config be options and fall back to checking env variables for any unset values sorta merging both constructors?
There was a problem hiding this comment.
I guess that depends on whether we should assume users always want to apply environment variable configuration. Personally I'd like know exactly where does the configuration comes from. But we could use environment variable to fill unset values in configs too.
There was a problem hiding this comment.
Yeah I see the pros and cons of both (e.g. the jaeger exporter has an explicit from_env). I think it is fine for now to get feedback from the community on which is preferred.
TommyCpp
force-pushed
the
env-var-config
branch
from
b2d13e1
to
a951e12
Compare
…ller than max queue size. * Add validation in `with_max_export_batch_size`, when input is larger than max queue size. Will lower it to be equal to max queue size. * Address comments.
TommyCpp
force-pushed
the
env-var-config
branch
from
a951e12
to
866047b
Compare
TommyCpp
force-pushed
the
env-var-config
branch
from
0785d01
to
9b83e31
Compare
part of #168