ronantakizawa/codereview-bench huggingface dataset:
Repository: open-telemetry/opentelemetry-java
File: exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/GrpcLogRecordExporterBuilderWrapper.java (Java)

Code before:
    return this;
  }

  @Override
  public TelemetryExporterBuilder<LogRecordData> setClientTls(
      byte[] privateKeyPem, byte[] certificatePem) {
    builder.setClientTls(privateKeyPem, certificatePem);
    return this;
  }

  @Override
  public TelemetryExporterBuilder<LogRecordData> setSslContext(
      SSLContext sslContext, X509TrustManager trustManager) {
    builder.setSslContext(sslContext, trustManager);
    return this;
  }

  @Override
  public TelemetryExporterBuilder<LogRecordData> setRetryPolicy(RetryPolicy retryPolicy) {
    builder.setRetryPolicy(retryPolicy);
    return this;
  }

  @Override
  public TelemetryExporterBuilder<LogRecordData> setProxyOptions(ProxyOptions proxyOptions) {
    throw new UnsupportedOperationException();
  }

  @Override
  @SuppressWarnings("deprecation") // testing deprecated functionality
  public TelemetryExporterBuilder<LogRecordData> setChannel(Object channel) {
    builder.setChannel((ManagedChannel) channel);
    return this;
  }

  @Override
  public TelemetryExporter<LogRecordData> build() {
    return TelemetryExporter.wrap(builder.build());
  }
}


Review comment (suggestion):
```suggestion
    throw new UnsupportedOperationException("ProxyOptions are not supported for gRPC");
```

Code after:
    builder.setTrustedCertificates(certificates);
    return this;
  }

  @Override
  public TelemetryExporterBuilder<LogRecordData> setClientTls(
      byte[] privateKeyPem, byte[] certificatePem) {
    builder.setClientTls(privateKeyPem, certificatePem);
    return this;
  }

  @Override
  public TelemetryExporterBuilder<LogRecordData> setSslContext(
      SSLContext sslContext, X509TrustManager trustManager) {
    builder.setSslContext(sslContext, trustManager);
    return this;
  }

  @Override
  public TelemetryExporterBuilder<LogRecordData> setRetryPolicy(RetryPolicy retryPolicy) {
    builder.setRetryPolicy(retryPolicy);
    return this;
  }

  @Override
  public TelemetryExporterBuilder<LogRecordData> setProxyOptions(ProxyOptions proxyOptions) {
    throw new UnsupportedOperationException("ProxyOptions are not supported for gRPC");
  }

  @Override
  @SuppressWarnings("deprecation") // testing deprecated functionality
  public TelemetryExporterBuilder<LogRecordData> setChannel(Object channel) {
    builder.setChannel((ManagedChannel) channel);
    return this;
  }

  @Override
  public TelemetryExporter<LogRecordData> build() {
    return TelemetryExporter.wrap(builder.build());
  }
}
Repository: open-telemetry/opentelemetry-java
File: exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/GrpcMetricExporterBuilderWrapper.java (Java)

Code before:
    return this;
  }

  @Override
  public TelemetryExporterBuilder<MetricData> setClientTls(
      byte[] privateKeyPem, byte[] certificatePem) {
    builder.setClientTls(privateKeyPem, certificatePem);
    return this;
  }

  @Override
  public TelemetryExporterBuilder<MetricData> setSslContext(
      SSLContext sslContext, X509TrustManager trustManager) {
    builder.setSslContext(sslContext, trustManager);
    return this;
  }

  @Override
  public TelemetryExporterBuilder<MetricData> setRetryPolicy(RetryPolicy retryPolicy) {
    builder.setRetryPolicy(retryPolicy);
    return this;
  }

  @Override
  public TelemetryExporterBuilder<MetricData> setProxyOptions(ProxyOptions proxyOptions) {
    throw new UnsupportedOperationException();
  }

  @Override
  @SuppressWarnings("deprecation") // testing deprecated functionality
  public TelemetryExporterBuilder<MetricData> setChannel(Object channel) {
    builder.setChannel((ManagedChannel) channel);
    return this;
  }

  @Override
  public TelemetryExporter<MetricData> build() {
    return TelemetryExporter.wrap(builder.build());
  }
}


Review comment (suggestion):
```suggestion
    throw new UnsupportedOperationException("ProxyOptions are not supported for gRPC");
```

Code after:
    builder.setTrustedCertificates(certificates);
    return this;
  }

  @Override
  public TelemetryExporterBuilder<MetricData> setClientTls(
      byte[] privateKeyPem, byte[] certificatePem) {
    builder.setClientTls(privateKeyPem, certificatePem);
    return this;
  }

  @Override
  public TelemetryExporterBuilder<MetricData> setSslContext(
      SSLContext sslContext, X509TrustManager trustManager) {
    builder.setSslContext(sslContext, trustManager);
    return this;
  }

  @Override
  public TelemetryExporterBuilder<MetricData> setRetryPolicy(RetryPolicy retryPolicy) {
    builder.setRetryPolicy(retryPolicy);
    return this;
  }

  @Override
  public TelemetryExporterBuilder<MetricData> setProxyOptions(ProxyOptions proxyOptions) {
    throw new UnsupportedOperationException("ProxyOptions are not supported for gRPC");
  }

  @Override
  @SuppressWarnings("deprecation") // testing deprecated functionality
  public TelemetryExporterBuilder<MetricData> setChannel(Object channel) {
    builder.setChannel((ManagedChannel) channel);
    return this;
  }

  @Override
  public TelemetryExporter<MetricData> build() {
    return TelemetryExporter.wrap(builder.build());
  }
}
Repository: open-telemetry/opentelemetry-java
File: exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/GrpcSpanExporterBuilderWrapper.java (Java)

Code before:
    return this;
  }

  @Override
  public TelemetryExporterBuilder<SpanData> setClientTls(
      byte[] privateKeyPem, byte[] certificatePem) {
    builder.setClientTls(privateKeyPem, certificatePem);
    return this;
  }

  @Override
  public TelemetryExporterBuilder<SpanData> setSslContext(
      SSLContext sslContext, X509TrustManager trustManager) {
    builder.setSslContext(sslContext, trustManager);
    return this;
  }

  @Override
  public TelemetryExporterBuilder<SpanData> setRetryPolicy(RetryPolicy retryPolicy) {
    builder.setRetryPolicy(retryPolicy);
    return this;
  }

  @Override
  public TelemetryExporterBuilder<SpanData> setProxyOptions(ProxyOptions proxyOptions) {
    throw new UnsupportedOperationException();
  }

  @Override
  @SuppressWarnings("deprecation") // testing deprecated functionality
  public TelemetryExporterBuilder<SpanData> setChannel(Object channel) {
    builder.setChannel((ManagedChannel) channel);
    return this;
  }

  @Override
  public TelemetryExporter<SpanData> build() {
    return TelemetryExporter.wrap(builder.build());
  }
}


Review comment (suggestion):
```suggestion
    throw new UnsupportedOperationException("ProxyOptions are not supported for gRPC");
```

Code after:
    builder.setTrustedCertificates(certificates);
    return this;
  }

  @Override
  public TelemetryExporterBuilder<SpanData> setClientTls(
      byte[] privateKeyPem, byte[] certificatePem) {
    builder.setClientTls(privateKeyPem, certificatePem);
    return this;
  }

  @Override
  public TelemetryExporterBuilder<SpanData> setSslContext(
      SSLContext sslContext, X509TrustManager trustManager) {
    builder.setSslContext(sslContext, trustManager);
    return this;
  }

  @Override
  public TelemetryExporterBuilder<SpanData> setRetryPolicy(RetryPolicy retryPolicy) {
    builder.setRetryPolicy(retryPolicy);
    return this;
  }

  @Override
  public TelemetryExporterBuilder<SpanData> setProxyOptions(ProxyOptions proxyOptions) {
    throw new UnsupportedOperationException("ProxyOptions are not supported for gRPC");
  }

  @Override
  @SuppressWarnings("deprecation") // testing deprecated functionality
  public TelemetryExporterBuilder<SpanData> setChannel(Object channel) {
    builder.setChannel((ManagedChannel) channel);
    return this;
  }

  @Override
  public TelemetryExporter<SpanData> build() {
    return TelemetryExporter.wrap(builder.build());
  }
}
Repository: open-telemetry/opentelemetry-java
File: sdk/trace/src/main/java/io/opentelemetry/sdk/trace/data/ExceptionEventData.java (Java)

Code before:
/*
 * Copyright The OpenTelemetry Authors
 * SPDX-License-Identifier: Apache-2.0
 */

package io.opentelemetry.sdk.trace.data;

import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.sdk.trace.SpanLimits;

/** Data representation of an event for a recorded exception. */
public interface ExceptionEventData extends EventData {

  /**
   * Returns a new immutable {@link ExceptionEventData}.
   *
   * @param spanLimits limits applied to {@link ExceptionEventData}.
   * @param epochNanos epoch timestamp in nanos of the {@link ExceptionEventData}.
   * @param exception the {@link Throwable exception} of the {@code Event}.
   * @param additionalAttributes the additional attributes of the {@link ExceptionEventData}.
   * @return a new immutable {@link ExceptionEventData}
   */
  static ExceptionEventData create(
      SpanLimits spanLimits,
      long epochNanos,
      Throwable exception,
      Attributes additionalAttributes) {
    return ImmutableExceptionEventData.create(
        spanLimits, epochNanos, exception, additionalAttributes);
  }

  /**
   * Return the {@link Throwable exception} of the {@link ExceptionEventData}.
   *
   * @return the {@link Throwable exception} of the {@link ExceptionEventData}
   */

Review comment (suggestion):
```suggestion
import javax.annotation.concurrent.Immutable;

/** Data representation of an event for a recorded exception. */
@Immutable
```

Code after:
/*
 * Copyright The OpenTelemetry Authors
 * SPDX-License-Identifier: Apache-2.0
 */

package io.opentelemetry.sdk.trace.data;

import io.opentelemetry.api.common.Attributes;
import javax.annotation.concurrent.Immutable;

/** Data representation of an event for a recorded exception. */
@Immutable
public interface ExceptionEventData extends EventData {

  /**
   * Returns a new immutable {@link ExceptionEventData}.
   *
   * @param epochNanos epoch timestamp in nanos of the {@link ExceptionEventData}.
   * @param exception the {@link Throwable exception} of the {@code Event}.
   * @param attributes the additional attributes of the {@link ExceptionEventData}.
   * @param totalAttributeCount the total number of attributes for this {@code} Event.
   * @return a new immutable {@link ExceptionEventData}
   */
  static ExceptionEventData create(
      long epochNanos, Throwable exception, Attributes attributes, int totalAttributeCount) {
    return ImmutableExceptionEventData.create(
        epochNanos, exception, attributes, totalAttributeCount);
  }

  /**
   * Return the {@link Throwable exception} of the {@link ExceptionEventData}.
   *
   * @return the {@link Throwable exception} of the {@link ExceptionEventData}
   */
  Throwable getException();
}
Repository: open-telemetry/opentelemetry-java
File: sdk/trace/src/main/java/io/opentelemetry/sdk/trace/data/ImmutableExceptionEventData.java (Java)

Code before:
import javax.annotation.concurrent.Immutable;

/** An effectively immutable implementation of {@link ExceptionEventData}. */
@AutoValue
@Immutable
abstract class ImmutableExceptionEventData implements ExceptionEventData {

  private static final String EXCEPTION_EVENT_NAME = "exception";

  @Override
  public final String getName() {
    return EXCEPTION_EVENT_NAME;
  }

  /**
   * Returns a new immutable {@code Event}.
   *
   * @param epochNanos epoch timestamp in nanos of the {@code Event}.
   * @param exception the {@link Throwable exception} of the {@code Event}.
   * @param additionalAttributes the additional {@link Attributes} of the {@code Event}.
   * @return a new immutable {@code Event<T>}
   */
  static ExceptionEventData create(
      long epochNanos, Throwable exception, Attributes additionalAttributes) {

    return create(epochNanos, exception, additionalAttributes, additionalAttributes.size());
  }

  /**
   * Returns a new immutable {@code Event}.
   *
   * @param epochNanos epoch timestamp in nanos of the {@code Event}.
   * @param exception the {@link Throwable exception} of the {@code Event}.
   * @param attributes the additional {@link Attributes} of the {@code Event}.
   * @param totalAttributeCount the total number of attributes for this {@code} Event.
   * @return a new immutable {@code Event<T>}
   */
  static ExceptionEventData create(
      long epochNanos, Throwable exception, Attributes attributes, int totalAttributeCount) {

    return new AutoValue_ImmutableExceptionEventData(
        attributes, epochNanos, totalAttributeCount, exception);
  }

  ImmutableExceptionEventData() {}
}


Review comment (suggestion):
```suggestion
  static ExceptionEventData create(
      long epochNanos, Throwable exception, Attributes additionalAttributes) {
    return create(epochNanos, exception, additionalAttributes, additionalAttributes.size());
```

Code after:
import javax.annotation.concurrent.Immutable;

/** An effectively immutable implementation of {@link ExceptionEventData}. */
@AutoValue
@Immutable
abstract class ImmutableExceptionEventData implements ExceptionEventData {

  private static final String EXCEPTION_EVENT_NAME = "exception";

  @Override
  public final String getName() {
    return EXCEPTION_EVENT_NAME;
  }

  /**
   * Returns a new immutable {@code Event}.
   *
   * @param epochNanos epoch timestamp in nanos of the {@code Event}.
   * @param exception the {@link Throwable exception} of the {@code Event}.
   * @param attributes the additional {@link Attributes} of the {@code Event}.
   * @param totalAttributeCount the total number of attributes for this {@code} Event.
   * @return a new immutable {@code Event<T>}
   */
  static ExceptionEventData create(
      long epochNanos, Throwable exception, Attributes attributes, int totalAttributeCount) {
    return new AutoValue_ImmutableExceptionEventData(
        attributes, epochNanos, totalAttributeCount, exception);
  }

  ImmutableExceptionEventData() {}
}
Repository: open-telemetry/opentelemetry-java
File: exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServer.java (Java)

Code before:
        };
    Thread shutdownThread = new Thread(shutdownFunction, "prometheus-httpserver-shutdown");
    shutdownThread.setDaemon(true);
    shutdownThread.start();
    return result;
  }

  @Override
  public void close() {
    shutdown().join(10, TimeUnit.SECONDS);
  }

  @Override
  public String toString() {
    StringJoiner joiner = new StringJoiner(",", "PrometheusHttpServer{", "}");
    joiner.add("host=" + host);
    joiner.add("port=" + port);
    joiner.add("otelScopeEnabled=" + otelScopeEnabled);
    joiner.add("allowedResourceAttributesFilter=" + allowedResourceAttributesFilter);
    joiner.add("memoryMode=" + memoryMode);
    joiner.add(
        "defaultAggregationSelector="
            + DefaultAggregationSelector.asString(defaultAggregationSelector));
    return joiner.toString();
  }

  /**
   * Returns a new {@link PrometheusHttpServerBuilder} with the same configuration as this instance.
   */
  public PrometheusHttpServerBuilder toBuilder() {
    return new PrometheusHttpServerBuilder(builder);
  }

  // Visible for testing.
  InetSocketAddress getAddress() {
    return new InetSocketAddress(host, httpServer.getPort());
  }
}


Review comment (bug):
This PR also improves the `PrometheusHttpServer#toString()` implementation, which has been failing to include the various configurable parameters.

Code after:
            result.fail();
          }
        };
    Thread shutdownThread = new Thread(shutdownFunction, "prometheus-httpserver-shutdown");
    shutdownThread.setDaemon(true);
    shutdownThread.start();
    return result;
  }

  @Override
  public void close() {
    shutdown().join(10, TimeUnit.SECONDS);
  }

  @Override
  public String toString() {
    StringJoiner joiner = new StringJoiner(",", "PrometheusHttpServer{", "}");
    joiner.add("host=" + host);
    joiner.add("port=" + port);
    joiner.add("otelScopeEnabled=" + otelScopeEnabled);
    joiner.add("allowedResourceAttributesFilter=" + allowedResourceAttributesFilter);
    joiner.add("memoryMode=" + memoryMode);
    joiner.add(
        "defaultAggregationSelector="
            + DefaultAggregationSelector.asString(defaultAggregationSelector));
    return joiner.toString();
  }

  /**
   * Returns a new {@link PrometheusHttpServerBuilder} with the same configuration as this instance.
   */
  public PrometheusHttpServerBuilder toBuilder() {
    return new PrometheusHttpServerBuilder(builder);
  }

  // Visible for testing.
  InetSocketAddress getAddress() {
    return new InetSocketAddress(host, httpServer.getPort());
  }
}
Repository: open-telemetry/opentelemetry-java
File: context/src/main/java/io/opentelemetry/context/propagation/TextMapGetter.java (Java)

Code before:
   * @since 0.10.0
   */
  Iterable<String> keys(C carrier);

  /**
   * Returns the first value of the given propagation {@code key} or returns {@code null}.
   *
   * @param carrier carrier of propagation fields, such as an http request.
   * @param key the key of the field.
   * @return the first value of the given propagation {@code key} or returns {@code null}.
   */
  @Nullable
  String get(@Nullable C carrier, String key);

  /**
   * If implemented, returns all values for a given {@code key} in order, or returns an empty list.
   *
   * <p>The default method returns the first value of the given propagation {@code key} as a
   * singleton list, or returns an empty list.
   *
   * @param carrier carrier of propagation fields, such as an http request.
   * @param key the key of the field.
   * @return the first value of the given propagation {@code key} as a singleton list, or returns an
   *     empty list.
   */
  default List<String> getList(@Nullable C carrier, String key) {
    String first = get(carrier, key);
    return first == null ? Collections.emptyList() : Collections.singletonList(first);
  }
}


Review comment (suggestion):
Perhaps using `List<String>` isn't the best option. Maybe `Iterable<String>` as used in `keys` or `Iterator<String>` would work better? When using `List<String>` and carrier is `HttpServletRequest` where `getHeaders` returns `Enumeration` you'd have to copy the enumeration to list with `Collections.list(carrier.getHeaders(key))`. When using `Iterator<String>` you could skip copying the enumeration to list with
```java
Enumeration<String> enumeration = carrier.getHeaders(key);
return new Iterator<String>() {
  @Override
  public boolean hasNext() {
    return enumeration.hasMoreElements();
  }

  @Override
  public String next() {
    return enumeration.nextElement();
  }
};
```

Code after:
/*
 * Copyright The OpenTelemetry Authors
 * SPDX-License-Identifier: Apache-2.0
 */

package io.opentelemetry.context.propagation;

import javax.annotation.Nullable;

/**
 * Interface that allows a {@code TextMapPropagator} to read propagated fields from a carrier.
 *
 * <p>{@code Getter} is stateless and allows to be saved as a constant to avoid runtime allocations.
 *
 * @param <C> carrier of propagation fields, such as an http request.
 */
public interface TextMapGetter<C> {

  /**
   * Returns all the keys in the given carrier.
   *
   * @param carrier carrier of propagation fields, such as an http request.
   * @since 0.10.0
   */
  Iterable<String> keys(C carrier);

  /**
   * Returns the first value of the given propagation {@code key} or returns {@code null}.
   *
   * @param carrier carrier of propagation fields, such as an http request.
   * @param key the key of the field.
   * @return the first value of the given propagation {@code key} or returns {@code null}.
   */
  @Nullable
  String get(@Nullable C carrier, String key);
}
Repository: open-telemetry/opentelemetry-java
File: context/src/main/java/io/opentelemetry/context/propagation/TextMapGetter.java (Java)

Code before:
   * @since 0.10.0
   */
  Iterable<String> keys(C carrier);

  /**
   * Returns the first value of the given propagation {@code key} or returns {@code null}.
   *
   * @param carrier carrier of propagation fields, such as an http request.
   * @param key the key of the field.
   * @return the first value of the given propagation {@code key} or returns {@code null}.
   */
  @Nullable
  String get(@Nullable C carrier, String key);

  /**
   * If implemented, returns all values for a given {@code key} in order, or returns an empty list.
   *
   * <p>The default method returns the first value of the given propagation {@code key} as a
   * singleton list, or returns an empty list.
   *
   * @param carrier carrier of propagation fields, such as an http request.
   * @param key the key of the field.
   * @return the first value of the given propagation {@code key} as a singleton list, or returns an
   *     empty list.
   */
  default List<String> getList(@Nullable C carrier, String key) {
    String first = get(carrier, key);
    return first == null ? Collections.emptyList() : Collections.singletonList(first);
  }
}


Review comment (suggestion):
It might be necessary for us to add this in an experimental fashion, rather than jumping straight to the stable API.

For example, we could have an `ExtendedTextMapGetter` interface in `opentelemetry-api-incubator` or in an internal package of `opentelemetry-context`. Then, `W3CBaggagePropagator` could check `if (getter instanceOf ExtendedTextMapGetter)` and adjust its behavior accordingly.

If we do this, I think it would have to go in `opentelemetry-context` instead of `opentelemetry-api-incubator` since `W3CBaggagePropagator` doesn't have access to `opentelemetry-api-incubator`.

Code after:
/*
 * Copyright The OpenTelemetry Authors
 * SPDX-License-Identifier: Apache-2.0
 */

package io.opentelemetry.context.propagation;

import javax.annotation.Nullable;

/**
 * Interface that allows a {@code TextMapPropagator} to read propagated fields from a carrier.
 *
 * <p>{@code Getter} is stateless and allows to be saved as a constant to avoid runtime allocations.
 *
 * @param <C> carrier of propagation fields, such as an http request.
 */
public interface TextMapGetter<C> {

  /**
   * Returns all the keys in the given carrier.
   *
   * @param carrier carrier of propagation fields, such as an http request.
   * @since 0.10.0
   */
  Iterable<String> keys(C carrier);

  /**
   * Returns the first value of the given propagation {@code key} or returns {@code null}.
   *
   * @param carrier carrier of propagation fields, such as an http request.
   * @param key the key of the field.
   * @return the first value of the given propagation {@code key} or returns {@code null}.
   */
  @Nullable
  String get(@Nullable C carrier, String key);
}