-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
sdk/trace: ParentBased is a decorator #4565
Comments
We should update the specification first https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/sdk.md#parentbased |
Huh? It is literally a decision hierarchy tree that determines what sampler to use. opentelemetry-go/sdk/trace/sampling.go Lines 266 to 282 in 1410496
Also Go is not an object-oriented language, I do not support including object-oriented terms in our descriptions. |
Even though GoF design patterns are coming from object-oriented languages (C++ and Smalltalk), Go is quite close to these languages (at least a lot closer than to functional languages). Even though there are key differences they are not related to the GoF design patterns which can be applied. PS. I would prefer using a more Go idiomatic term instead but I am not aware if such exist. |
ParentBased sampler is decorator (and not a composite). > The intent of a composite is to "compose" objects into tree structures to represent part-whole hierarchies. > What problems can the Composite design pattern solve? > [...] > A part-whole hierarchy should be represented as tree structure `ParentBased` does not produce a tree. Composite aims to solve a different (but kind of similar) problem. > decorator pattern [...] that allows behavior to be added to an individual [object](https://en.wikipedia.org/wiki/Object_(computer_science)), dynamically, without affecting the behavior of other objects from the same [class](https://en.wikipedia.org/wiki/Class_(computer_science)). > What problems can it solve? > Responsibilities should be added to (and removed from) an object dynamically at run-time.[5] >>A flexible alternative to subclassing for extending functionality should be provided. The key feature of the `ParentBased` is how it changes the "behavior" of the wrapped sampler. We decorate the existing sampler so that it handles the sampling based on the parent's span. References: - https://en.wikipedia.org/wiki/Decorator_pattern - https://en.wikipedia.org/wiki/Composite_pattern - https://sourcemaking.com/design_patterns/composite - https://sourcemaking.com/design_patterns/decorator - https://stackoverflow.com/questions/2233952/difference-between-the-composite-pattern-and-decorator-pattern Related issue open-telemetry/opentelemetry-go#4565
From open-telemetry/opentelemetry.io@5847bd3#r128534150
Update the docs and describe ParentBased sampler as a decorator (not composite).
The text was updated successfully, but these errors were encountered: