Extend HttpServerResponseMutator with replace and set if absent variants

[ddark008]

Is your feature request related to a problem? Please describe.

Current HttpServerResponseMutator provides only appending header values. In some circumstances, this can lead to duplicate header values. For example, the instrumentation of the spring-cloud-gateway proxy adds additional headers to all packets that are passed through, in addition to existing headers.

Describe the solution you'd like

Extend HttpServerResponseMutator with new methods:

• replace existing headers
• set header if absent

Describe alternatives you've considered

No response

Additional context

I can provide some PR to support these features

[steverao]

It looks good! @open-telemetry/java-instrumentation-approvers WDYT?

[laurit]

I think this sounds reasonable. It will require some thought to determine which methods to add. For example if one wishes to modify the value for a header that can contain a comma separated list of values like https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Expose-Headers it would be handy to have access to the original value so instead of overwriting one could append to the existing value. Things are further complicated by the possibility of having the same header set multiple times.

[ddark008]

Hi, @laurit & @steverao

I suggest adding the following features:

• Replace an existing header value or add a new one if it doesn't exist.
• Remove a specific header by its name.
• Retrieve a header's values.
• Retrieve all request headers and their values.

If you are satisfied with this, I will do a PR with the implementation of the new interface:

/** Provides the ability to mutate an instrumentation library specific response. */
public interface HttpServerResponseMutator<RESPONSE> {
  /**
   * Appends a value to the list of values for the specified header.
   *
   * @param response the response object
   * @param name the name of the header
   * @param value the value to append to the header
   */
  void appendHeader(RESPONSE response, String name, String value);
  /**
   * Sets a header to the specified value, replacing any existing values.
   *
   * @param response the response object
   * @param name the name of the header
   * @param value the value to set for the header
   */
  void setHeader(RESPONSE response, String name, String value);
  /**
   * Sets a header to the specified values, replacing any existing values.
   *
   * @param response the response object
   * @param name the name of the header
   * @param values the values to set for the header
   */
  void setHeader(RESPONSE response, String name, List<String> values);
  /**
   * Removes a header from the response.
   *
   * @param response the response object
   * @param name the name of the header to remove
   */
  void removeHeader(RESPONSE response, String name);
  /**
   * Returns the values of headers with the specified name.
   *
   * @param response the response object
   * @param name the name of the header
   * @return A {@link List} of header values which will be empty if no values are found
   */
  List<String> getHeader(RESPONSE response, String name);
  /**
   * Retrieves all headers as a map.
   *
   * @param response the response object
   * @return a {@link Map} of header names and their associated {@link List} of values both will be empty if no values are found
   */
  Map<String, List<String>> getAllHeaders(RESPONSE response);
}

[trask]

what do you think is the minimum set of methods we need?

[laurit]

I have nothing against implementing this but you have to keep in mind that there are multiple implementations for this interface. Adding these methods is probably easy for most of the HttpServerResponseMutator but there is probably going to be at least one where this is going to be tricky (have a look at https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/instrumentation/akka/akka-http-10.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/akkahttp/server/AkkaHttpResponseMutator.java). If this is something you need I'd suggest you to give a try at implementing this, don't forget the tests.

[github-actions]

Since there has been no activity on this enhancement for the past year we are closing it to help maintain our backlog. Anyone who would like to work on it is still welcome to do so, and we can re-open it at that time.