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

Suggestions on how to filter Attributes now that forEach takes BiConsumer? #2283

Closed
trask opened this issue Dec 12, 2020 · 5 comments
Closed
Labels
Feature Request Suggest an idea for this project

Comments

@trask
Copy link
Member

trask commented Dec 12, 2020

Filtering Attributes in a SpanProcessor used to be simpler when Attributes.forEach() had strong typing via AttributeConsumer.accept(AttributeKey<T>, T).

E.g. removing an attribute previously:

AttributesBuilder builder = Attributes.builder();
attributes.forEach(
    new AttributeConsumer() {
      @Override
      public <T> void accept(AttributeKey<T> key, T value) {
        if (!key.getKey().equals("mykey")) {
          builder.put(key, value);
        }
      }
    });

this same approach is now longer and requires @SuppressWarnings("unchecked"):

AttributesBuilder builder = Attributes.builder();
attributes.forEach(
    (key, value) -> {
      if (!key.getKey().equals("mykey")) {
        switch (key.getType()) {
          case STRING:
            builder.put((AttributeKey<String>) key, (String) value);
            break;
          case LONG:
            builder.put((AttributeKey<Long>) key, (Long) value);
            break;
          case BOOLEAN:
            builder.put((AttributeKey<Boolean>) key, (Boolean) value);
            break;
          case DOUBLE:
            builder.put((AttributeKey<Double>) key, (Double) value);
            break;
          case STRING_ARRAY:
          case LONG_ARRAY:
          case BOOLEAN_ARRAY:
          case DOUBLE_ARRAY:
            builder.put((AttributeKey<List<?>>) key, (List<?>) value);
            break;
          default:
            // not resilient to new types being added?
        }
      }
    });

Is the recommendation now to create a DelegatingAttributes similar to DelegatingSpanData that performs the filtering? (I'm going to try this out and will report back)

@trask trask added the Feature Request Suggest an idea for this project label Dec 12, 2020
@trask
Copy link
Member Author

trask commented Dec 12, 2020

Here's a rough sketch of using delegate approach to remove a key. This would be a bit nicer with default methods on AttributeBuilder, similar to what #2242 did for Span.

class WithoutKeyAttributes implements Attributes {

    private final Attributes delegate;
    private final AttributeKey<?> withoutKey;

    WithoutKeyAttributes(Attributes delegate, AttributeKey<?> withoutKey) {
        if (delegate.get(withoutKey) == null) {
            throw new IllegalStateException("size() and isEmpty() implementation assumes delegate contains key," +
                    " no need to wrap if it doesn't contain key anyways");
        }
        this.delegate = delegate;
        this.withoutKey = withoutKey;
    }

    @Override
    public <T> T get(AttributeKey<T> key) {
        if (key.equals(withoutKey)) {
            return null;
        }
        return delegate.get(key);
    }

    @Override
    public void forEach(BiConsumer<AttributeKey<?>, Object> consumer) {
        delegate.forEach((key, value) -> {
            if (!key.equals(withoutKey)) {
                consumer.accept(key, value);
            }
        });
    }

    @Override
    public int size() {
        return delegate.size() - 1;
    }

    @Override
    public boolean isEmpty() {
        return size() == 0;
    }

    @Override
    public Map<AttributeKey<?>, Object> asMap() {
        // TODO could wrap the map from delegate
        HashMap<AttributeKey<?>, Object> copy = new HashMap<>(delegate.asMap());
        copy.remove(withoutKey);
        return copy;
    }

    @Override
    public AttributesBuilder toBuilder() {
        return new WithoutKeyAttributesBuilder(delegate.toBuilder(), withoutKey);
    }
}
class WithoutKeyAttributesBuilder implements AttributesBuilder {

    private final AttributesBuilder delegate;
    private final AttributeKey<?> withoutKey;
    private boolean keyPutBack;

    WithoutKeyAttributesBuilder(AttributesBuilder delegate, AttributeKey<?> withoutKey) {
        this.delegate = delegate;
        this.withoutKey = withoutKey;
    }

    @Override
    public Attributes build() {
        if (keyPutBack) {
            return delegate.build();
        } else {
            return new WithoutKeyAttributes(delegate.build(), withoutKey);
        }
    }

    @Override
    public <T> AttributesBuilder put(AttributeKey<Long> key, int value) {
        if (key.equals(withoutKey)) {
            keyPutBack = true;
        }
        return delegate.put(key, value);
    }

    @Override
    public <T> AttributesBuilder put(AttributeKey<T> key, T value) {
        if (key.equals(withoutKey)) {
            keyPutBack = true;
        }
        return delegate.put(key, value);
    }

    @Override
    public AttributesBuilder put(String key, String value) {
        if (key.equals(withoutKey.getKey())) {
            keyPutBack = true;
        }
        return delegate.put(key, value);
    }

    @Override
    public AttributesBuilder put(String key, long value) {
        if (key.equals(withoutKey.getKey())) {
            keyPutBack = true;
        }
        return delegate.put(key, value);
    }

    @Override
    public AttributesBuilder put(String key, double value) {
        if (key.equals(withoutKey.getKey())) {
            keyPutBack = true;
        }
        return delegate.put(key, value);
    }

    @Override
    public AttributesBuilder put(String key, boolean value) {
        if (key.equals(withoutKey.getKey())) {
            keyPutBack = true;
        }
        return delegate.put(key, value);
    }

    @Override
    public AttributesBuilder put(String key, String... value) {
        if (key.equals(withoutKey.getKey())) {
            keyPutBack = true;
        }
        return delegate.put(key, value);
    }

    @Override
    public AttributesBuilder put(String key, long... value) {
        if (key.equals(withoutKey.getKey())) {
            keyPutBack = true;
        }
        return delegate.put(key, value);
    }

    @Override
    public AttributesBuilder put(String key, double... value) {
        if (key.equals(withoutKey.getKey())) {
            keyPutBack = true;
        }
        return delegate.put(key, value);
    }

    @Override
    public AttributesBuilder put(String key, boolean... value) {
        if (key.equals(withoutKey.getKey())) {
            keyPutBack = true;
        }
        return delegate.put(key, value);
    }

    @Override
    public AttributesBuilder putAll(Attributes attributes) {
        if (attributes.get(withoutKey) != null) {
            keyPutBack = true;
        }
        return delegate.putAll(attributes);
    }
}

@anuraaga
Copy link
Contributor

Probably not a solution but you can suppress rawtypes as well and remove the type parameter in the first example and it would compile I think.

Another idea is providing a toAttributes collector to use with something like `attributes.asMap().entrySet().stream().filter(...).collect(toAttributes());

@trask
Copy link
Member Author

trask commented Dec 13, 2020

suppressing rawtypes is a good idea 👍

I'm not clear how attributes.asMap().entrySet().stream()... would help, b/c it loses the types also

@anuraaga
Copy link
Contributor

toAttributes would handle the casting inside to hide it from user. Probably can't use rawtypes there since we would need to do some type checks.

@trask
Copy link
Member Author

trask commented Jan 29, 2021

Closing this, thanks for the suggestions 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request Suggest an idea for this project
Projects
None yet
Development

No branches or pull requests

2 participants