Skip to content

Convert external-annotations javaagent tests from groovy to java#9557

Merged
trask merged 3 commits into
open-telemetry:mainfrom
jaydeluca:external-annotations-conversion
Sep 29, 2023
Merged

Convert external-annotations javaagent tests from groovy to java#9557
trask merged 3 commits into
open-telemetry:mainfrom
jaydeluca:external-annotations-conversion

Conversation

@jaydeluca

@jaydeluca jaydeluca commented Sep 26, 2023

Copy link
Copy Markdown
Member

Related to #7195

I was also looking at the javaagent-unit-tests, but the existing test in groovy accesses a method that is not visible in Java. I am trying to think of a way to rethink the approach to the test, but open to suggestions if anyone can think of a way

@jaydeluca jaydeluca requested a review from a team September 26, 2023 01:24
InvocationTargetException {

Class<?> cls =
Class.forName("io.opentelemetry.javaagent.instrumentation.extannotations.SayTracedHello");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think you don't need the Class.forName here, could directly use SayTraceHello.class. Or you could do something like

  @ParameterizedTest
  @EnumSource(TraceProvider.class)
  void testShouldSupportProvider(TraceProvider provider) {
    provider.test();

    testing.waitAndAssertTraces(
        trace ->
            trace.hasSpansSatisfyingExactly(
                span ->
                    span.hasName("SayTracedHello." + provider.testMethodName())
                        .hasAttributesSatisfyingExactly(
                            equalTo(
                                SemanticAttributes.CODE_NAMESPACE, SayTracedHello.class.getName()),
                            equalTo(SemanticAttributes.CODE_FUNCTION, provider.testMethodName()),
                            equalTo(stringKey("providerAttr"), provider.name()))));
  }

  @SuppressWarnings("ImmutableEnumChecker")
  private enum TraceProvider {
    AppOptics((sayTracedHello) -> sayTracedHello.appoptics()),
    Datadog((sayTracedHello) -> sayTracedHello.datadog()),
    Dropwizard((sayTracedHello) -> sayTracedHello.dropwizard()),
    KamonOld((sayTracedHello) -> sayTracedHello.kamonold()),
    KamonNew((sayTracedHello) -> sayTracedHello.kamonnew()),
    NewRelic((sayTracedHello) -> sayTracedHello.newrelic()),
    SignalFx((sayTracedHello) -> sayTracedHello.signalfx()),
    Sleuth((sayTracedHello) -> sayTracedHello.sleuth()),
    Tracelytics((sayTracedHello) -> sayTracedHello.tracelytics());

    final Consumer<SayTracedHello> test;

    TraceProvider(Consumer<SayTracedHello> test) {
      this.test = test;
    }

    void test() {
      test.accept(new SayTracedHello());
    }

    String testMethodName() {
      return name().toLowerCase(Locale.ROOT);
    }
  }

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤯 that is a much more elegant approach, thank you!

@laurit

laurit commented Sep 29, 2023

Copy link
Copy Markdown
Contributor

I was also looking at the javaagent-unit-tests, but the existing test in groovy accesses a method that is not visible in Java. I am trying to think of a way to rethink the approach to the test, but open to suggestions if anyone can think of a way

The usual solution is to make that method package private and add // visible for testing and have the test in the same package.

@trask trask merged commit db9b628 into open-telemetry:main Sep 29, 2023
@jaydeluca jaydeluca deleted the external-annotations-conversion branch October 11, 2023 10:50
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.

3 participants