Skip to content

Commit 285c32d

Browse files
authored
added a section on configuration to docs/maintainers.md (#54)
1 parent e62dc08 commit 285c32d

File tree

1 file changed

+54
-1
lines changed

1 file changed

+54
-1
lines changed

doc/maintainers.md

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,55 @@ event loop instead of a dedicated thread.
301301
event dispatch facilities.
302302
303303
### Configuration
304-
TODO
304+
There's a good [blog post][32] by [Alexis King][31] where she makes the case for
305+
encoding configuration validation into the type system. Forbid invalid states by
306+
making configurable components accept a different type than that which is used
307+
to specify configuration.
308+
309+
This is not a new idea. It's been used, for example, to "taint" strings that
310+
originate as program inputs. Then you can't accidentally pass user-influenced
311+
inputs to, say, `std::system`, because `std::system` takes a `const char*`, not
312+
a `class UserTaintedString`. There's still ample opportunity to cast away the
313+
taint and sneak it into some string building operation, but at least `class
314+
UserTaintedString` gives hope that a static analysis tool could be used to fill
315+
in some gaps in human code review.
316+
317+
This library adopts that approach for configuration. The configuration of `class
318+
Tracer` is `class TracerConfig`, but in order to construct a `Tracer` you must
319+
first convert the `TracerConfig` into a `FinalizedTracerConfig` by calling
320+
`finalize_config`. If there is anything wrong with the `TracerConfig` or with
321+
environment variables that would override it, `finalize_config` will return an
322+
`Error` instead of a `FinalizedTracerConfig`. In that case, you can't create a
323+
`Tracer` at all.
324+
325+
This technique applies to multiple components:
326+
327+
| Component | Unvalidated | Validated | Parser |
328+
| --------------- | ----------- | --------- | ----------------- |
329+
| `Tracer` | `TracerConfig` | `FinalizedTracerConfig` | `finalize_config` in [tracer_config.h][10] |
330+
| `DatadogAgent` | `DatadogAgentConfig` | `FinalizedDatadogAgentConfig` | `finalize_config` in [datadog_agent_config.h][17] |
331+
| `TraceSampler` | `TraceSamplerConfig` | `FinalizedTraceSamplerConfig` | `finalize_config` in [trace_sampler_config.h][33] |
332+
| `SpanSampler` | `SpanSamplerConfig` | `FinalizedSpanSamplerConfig` | `finalize_config` in [span_sampler_config.h][34] |
333+
| multiple | `double` | `Rate` | `Rate::from` in [rate.h][35] |
334+
335+
An alternative approach, that Caleb espouses, is to accept invalid configuration
336+
quietly. When invalid configuration is detected, the library could substitute a
337+
reasonable default and then send notice of the configuration issue to Datadog,
338+
e.g. as a hidden span tag. That information would then be available to Support
339+
should the customer raise an issue due to a difference in behavior between what
340+
they see and what they think they configured.
341+
342+
This library uses the stricter approach. The downside is that a user of the
343+
library has to decide what to do when even the slightest part of the
344+
configuration or environment is deemed invalid.
345+
346+
One other convention of the library is that `FinalizedFooConfig` (for some
347+
`Foo`) is never a data member of the configured component class. That is,
348+
`FinalizedTracerConfig` is not stored in `Tracer`. Instead, a constructor might
349+
individually copy the finalized config's data members. This is to prevent
350+
eventual intermixing between the "configuration representation" and the "runtime
351+
representation." In part, `finalize_config` already mitigates the problem.
352+
Abstaining from storing the finalized config as a data member is a step further.
305353
306354
### Error Handling
307355
TODO
@@ -361,3 +409,8 @@ TODO
361409
[28]: https://github.com/DataDog/dd-trace-cpp/blob/ca155b3da65c2dc235cf64a28f8e0d8fdab3700c/src/datadog/datadog_agent_config.h#L50-L51
362410
[29]: https://github.com/DataDog/nginx-datadog/blob/master/src/ngx_event_scheduler.h
363411
[30]: https://github.com/envoyproxy/envoy/blob/main/source/extensions/tracers/datadog/event_scheduler.h
412+
[31]: https://lexi-lambda.github.io/about.html
413+
[32]: https://lexi-lambda.github.io/blog/2019/11/05/parse-don-t-validate/
414+
[33]: ../src/datadog/trace_sampler_config.h
415+
[34]: ../src/datadog/span_sampler_config.h
416+
[35]: ../src/datadog/rate.h

0 commit comments

Comments
 (0)