Skip to content
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

ParentBased is a decorator #3706

Merged
merged 2 commits into from
Sep 28, 2023
Merged

ParentBased is a decorator #3706

merged 2 commits into from
Sep 28, 2023

Conversation

pellared
Copy link
Member

@pellared pellared commented Sep 27, 2023

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, dynamically, without affecting the behavior of other objects from the same class.

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:

Related issue open-telemetry/opentelemetry-go#4565

@pellared pellared requested review from a team September 27, 2023 17:44
@yurishkuro
Copy link
Member

image

@MrAlias
Copy link
Contributor

MrAlias commented Sep 27, 2023

I am not in favor of using explicitly object-oriented language to describe things in the specification given many languages that implement it are not object-oriented. Also, the implementation of this sampler can quite literally be a decision hierarchy tree matching what you define as "composite". I think the current definition is correct.

@yurishkuro
Copy link
Member

yurishkuro commented Sep 27, 2023

Decorator is a design pattern that means "modifies behavior of underlying thing without changing its interface". A function can decorate another function, there's nothing inherently object-oriented about it.

In contrast, "composite" means containing more than one other things, and doesn't imply anything about the resulting interface. In the context of ParentBased sampler decorator is a better term.

Copy link
Member

@tigrannajaryan tigrannajaryan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a modification to a Stable doc. I believe we are allowed to do since it does not change any contract or behavior, it merely uses a more correct terminology.

pellared referenced this pull request in open-telemetry/opentelemetry.io Sep 27, 2023
@yurishkuro yurishkuro merged commit 89f5292 into open-telemetry:main Sep 28, 2023
6 checks passed
@pellared pellared deleted the patch-4 branch September 28, 2023 19:17
carlosalberto pushed a commit to carlosalberto/opentelemetry-specification that referenced this pull request Oct 31, 2024
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants