Skip to content

Simplified Declarative Configuration API, Option B#15672

Closed
trask wants to merge 2 commits intoopen-telemetry:mainfrom
trask:simplified-declarative-config-api-option-b
Closed

Simplified Declarative Configuration API, Option B#15672
trask wants to merge 2 commits intoopen-telemetry:mainfrom
trask:simplified-declarative-config-api-option-b

Conversation

@trask
Copy link
Copy Markdown
Member

@trask trask commented Dec 16, 2025

Exploring options for us to start using in this repository immediately, and to recommend upstream to the Declarative Configuration API.

  // instrumentation/development:
  //   java:
  //     graphql:
  //       capture_query: true
  //       query_sanitizer:
  //         enabled: true
  //       data_fetcher:
  //         enabled: false
  //       trivial_data_fetcher:
  //         enabled: false
  //       add_operation_name_to_span_name:
  //         enabled: false
  private static final class Configuration {

    private final boolean captureQuery;
    private final boolean sanitizeQuery;
    private final boolean dataFetcherEnabled;
    private final boolean trivialDataFetcherEnabled;
    private final boolean addOperationNameToSpanName;

    Configuration(OpenTelemetry openTelemetry) {
      ExtendedDeclarativeConfigProperties instrumentationConfig =
          DeclarativeConfigUtil.get(openTelemetry);

      this.captureQuery = instrumentationConfig.getBoolean("graphql", "capture_query").orElse(true);
      this.sanitizeQuery =
          instrumentationConfig.getBoolean("graphql", "query_sanitizer", "enabled").orElse(true);
      this.dataFetcherEnabled =
          instrumentationConfig.getBoolean("graphql", "data_fetcher", "enabled").orElse(false);
      this.trivialDataFetcherEnabled =
          instrumentationConfig
              .getBoolean("graphql", "trivial_data_fetcher", "enabled")
              .orElse(false);
      this.addOperationNameToSpanName =
          instrumentationConfig
              .getBoolean("graphql", "add_operation_name_to_span_name", "enabled")
              .orElse(false);
    }
  }

@trask trask force-pushed the simplified-declarative-config-api-option-b branch from 4d763a1 to 1e0678b Compare December 16, 2025 18:36
this.delegate = delegate;
}

public Optional<Boolean> getBoolean(String... path) {
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.

this is the new method in DeclarativeConfigProperties that is proposed in this PR

@trask trask force-pushed the simplified-declarative-config-api-option-b branch from 1e0678b to f0682ac Compare December 16, 2025 18:47
@trask trask marked this pull request as ready for review December 16, 2025 19:11
@trask trask requested a review from a team as a code owner December 16, 2025 19:11
@jack-berg
Copy link
Copy Markdown
Member

I prefer option A because supporting getBoolean("graphql", "query_sanitizer", "enabled") means a larger API surface area accommodating all the different types than get("graphql").get("query_sanitizer").getBoolean("enabled") and I think which is better in terms of UX is a bit of a tossup.

So without a clear advantage in UX, opt for simpler.

@trask
Copy link
Copy Markdown
Member Author

trask commented Dec 16, 2025

Closing in favor of #15671

@trask trask closed this Dec 16, 2025
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.

2 participants