Resurrects duplicate handlers and explains all the bugs we can make

[codefromthecrypt]

I really don't like this in any way.

[codefromthecrypt]

here's the problem that can sneak up on the next change to add a list parameter.

  @Configuration
  static class ListBindingProblems {
    @Autowired List<A> as;
    @Autowired List<B> bs;

    @Bean A provideA() {
      return new AB();
    }
  }

  @Test public void listBindingProblems() {
    context.register(ListBindingProblems.class);
    context.refresh();

    ListBindingProblems problems = context.getBean(ListBindingProblems.class);
    assertThat(problems.as).hasSize(1);
    // eventhough the user declared their bean method provideA as returning A, spring will look at
    // all types and magically fill here. What that means is you have to be defensive about dupes,
    // or make it impossible to create dupes by type hierarchy.
    assertThat(problems.bs).hasSize(1);
  }

[codefromthecrypt]

The concrete problem is we need to provide a means to affect propagation, and almost always people are adding tags only visible from finished span handler. For example, almost always if someone adds extra field "correlation-id" they are also adding it as a tag, and this has been asked for a lot (ex #577).

While incomplete, secondary sampling (#958) and secondary trace context (#693) have similar needs. Rather than create a new type that is the same as FinishedSpanHandler, accepts the same parameters etc, just to avoid dupes... or to make people using a plugin to have to register in two places to avoid implicit registration dupes, the easiest way out is to not allow dupes.

While it is very easy to come up with reasons for not allowing dupes, I've not heard any reason to allow dupes especially on object.equals. Optimizing towards allowing dupes opens up bugs and really complicates the ability to add features.

[anuraaga]

I don't feel that strongly about allowing dupes ;) Just that it feels when calling an add method multiple times it wouldn't silently ignore some. I see that Spring makes it easier to mess something up.

Instead of dropping dupes, what do you think about throwing an exception for a dupe so it's not just silently dropped?

[codefromthecrypt]

throwing an exception works for me. we could check the result of set.add for example

[codefromthecrypt]

problem with throwing an exception though is that if the dupes are outside their control you crash the app for no good reason. I think logging would be better, don't you?

[codefromthecrypt]

because the dupes still could arise from the same scenario described. many times people are not writing config directly rather inheriting it.

[codefromthecrypt]

basically there is no use case for dupes and I hate having to always do emergency releases for preventable things.. it is hard to get behind crashing over this

[codefromthecrypt]

merging as is because at least the work I am doing can avoid most of these bugs. also I want to reserve focus for more important code