Restore original Request ID functionality

This PR enacts the action items decided in the following ML thread:

https://lists.apache.org/thread/4gmlcn84o7n8d12qnpl7grfbm4zypk7b

The action items are:

1. Restore the original functionality for request IDs, change the
default header name back to x-request-id
2. Remove RequestIdGenerator and related functionality.
3. Update PolarisEvent, expose request ID if available, expose OTel
context if available.
4. Update events table SQL schema: insert request ID if available,
insert OTel context if available.

Furthermore, this PR also fixes #2913 and adds a small integration test for the JDBC events sink: `InMemoryBufferEventListenerIntegrationTest`.

Finally, it modifies the Helm chart to reflect the configuration changes.

Author: adutra

CHANGELOG.md

@@ -31,8 +31,17 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti
 
 ### Upgrade notes
 
+- JDBC persistence: a new column has been added to the `events` table to store the OpenTelemetry 
+  context that was active when the event was emitted. This column is nullable. To update your schema
+  to the latest version, run the following SQL statement: 
+  ```sql 
+  ALTER TABLE POLARIS_SCHEMA.EVENTS ADD COLUMN IF NOT EXISTS otel_context TEXT; 
+  ```
+
 ### Breaking changes
 
+- The default request ID header name has changed from `Polaris-Request-Id` to `x-request-id`.
+
 ### New Features
 
 - Support credential vending for federated catalogs. `ALLOW_FEDERATED_CATALOGS_CREDENTIAL_VENDING` (default: true) was added to toggle this feature.
@@ -43,6 +52,15 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti
 
 ### Deprecations
 
+* The configuration property `polaris.log.request-id-header-name` is deprecated and has been renamed 
+  to `polaris.tracing.request-id.header-name`; the old name is still supported for backwards 
+  compatibility, but will generate a warning. It will be removed in a future release.
+* Helm chart: the `tracing` section has been renamed to `tracing.otel`. The old name is still
+  supported for backwards compatibility, but will be removed in a future release.
+* Helm chart: the `logging.requestIdHeaderName` property is deprecated and has been renamed to 
+  `tracing.requestId.headerName`. The old name is still supported for backwards compatibility, but
+  will be removed in a future release.
+
 ### Fixes
 
 ### Commits

helm/polaris/README.md

@@ -247,7 +247,7 @@ ct install --namespace polaris --charts ./helm/polaris
 | livenessProbe.successThreshold | int | `1` | Minimum consecutive successes for the probe to be considered successful after having failed. Minimum value is 1. |
 | livenessProbe.terminationGracePeriodSeconds | int | `30` | Optional duration in seconds the pod needs to terminate gracefully upon probe failure. Minimum value is 1. |
 | livenessProbe.timeoutSeconds | int | `10` | Number of seconds after which the probe times out. Minimum value is 1. |
-| logging | object | `{"categories":{"org.apache.iceberg.rest":"INFO","org.apache.polaris":"INFO"},"console":{"enabled":true,"format":"%d{yyyy-MM-dd HH:mm:ss,SSS} %-5p [%c{3.}] [%X{requestId},%X{realmId}] [%X{traceId},%X{parentId},%X{spanId},%X{sampled}] (%t) %s%e%n","json":false,"threshold":"ALL"},"file":{"enabled":false,"fileName":"polaris.log","format":"%d{yyyy-MM-dd HH:mm:ss,SSS} %-5p [%c{3.}] [%X{requestId},%X{realmId}] [%X{traceId},%X{parentId},%X{spanId},%X{sampled}] (%t) %s%e%n","json":false,"logsDir":"/deployments/logs","rotation":{"fileSuffix":null,"maxBackupIndex":5,"maxFileSize":"100Mi"},"storage":{"className":"standard","selectorLabels":{},"size":"512Gi"},"threshold":"ALL"},"level":"INFO","mdc":{},"requestIdHeaderName":"Polaris-Request-Id"}` | Logging configuration. |
+| logging | object | `{"categories":{"org.apache.iceberg.rest":"INFO","org.apache.polaris":"INFO"},"console":{"enabled":true,"format":"%d{yyyy-MM-dd HH:mm:ss,SSS} %-5p [%c{3.}] [%X{requestId},%X{realmId}] [%X{traceId},%X{parentId},%X{spanId},%X{sampled}] (%t) %s%e%n","json":false,"threshold":"ALL"},"file":{"enabled":false,"fileName":"polaris.log","format":"%d{yyyy-MM-dd HH:mm:ss,SSS} %-5p [%c{3.}] [%X{requestId},%X{realmId}] [%X{traceId},%X{parentId},%X{spanId},%X{sampled}] (%t) %s%e%n","json":false,"logsDir":"/deployments/logs","rotation":{"fileSuffix":null,"maxBackupIndex":5,"maxFileSize":"100Mi"},"storage":{"className":"standard","selectorLabels":{},"size":"512Gi"},"threshold":"ALL"},"level":"INFO","mdc":{}}` | Logging configuration. |
 | logging.categories | object | `{"org.apache.iceberg.rest":"INFO","org.apache.polaris":"INFO"}` | Configuration for specific log categories. |
 | logging.console | object | `{"enabled":true,"format":"%d{yyyy-MM-dd HH:mm:ss,SSS} %-5p [%c{3.}] [%X{requestId},%X{realmId}] [%X{traceId},%X{parentId},%X{spanId},%X{sampled}] (%t) %s%e%n","json":false,"threshold":"ALL"}` | Configuration for the console appender. |
 | logging.console.enabled | bool | `true` | Whether to enable the console appender. |
@@ -271,7 +271,6 @@ ct install --namespace polaris --charts ./helm/polaris
 | logging.file.threshold | string | `"ALL"` | The log level of the file appender. |
 | logging.level | string | `"INFO"` | The log level of the root category, which is used as the default log level for all categories. |
 | logging.mdc | object | `{}` | Configuration for MDC (Mapped Diagnostic Context). Values specified here will be added to the log context of all incoming requests and can be used in log patterns. |
-| logging.requestIdHeaderName | string | `"Polaris-Request-Id"` | The header name to use for the request ID. |
 | managementService | object | `{"annotations":{},"clusterIP":"None","externalTrafficPolicy":null,"internalTrafficPolicy":null,"ports":[{"name":"polaris-mgmt","nodePort":null,"port":8182,"protocol":null,"targetPort":null}],"sessionAffinity":null,"trafficDistribution":null,"type":"ClusterIP"}` | Management service settings. These settings are used to configure liveness and readiness probes, and to configure the dedicated headless service that will expose health checks and metrics, e.g. for metrics scraping and service monitoring. |
 | managementService.annotations | object | `{}` | Annotations to add to the service. |
 | managementService.clusterIP | string | `"None"` | By default, the management service is headless, i.e. it does not have a cluster IP. This is generally the right option for exposing health checks and metrics, e.g. for metrics scraping and service monitoring. |
@@ -368,7 +367,10 @@ ct install --namespace polaris --charts ./helm/polaris
 | tasks.maxConcurrentTasks | string | `nil` | The maximum number of concurrent tasks that can be executed at the same time. The default is the number of available cores. |
 | tasks.maxQueuedTasks | string | `nil` | The maximum number of tasks that can be queued up for execution. The default is Integer.MAX_VALUE. |
 | tolerations | list | `[]` | A list of tolerations to apply to polaris pods. See https://kubernetes.io/docs/concepts/scheduling-eviction/taint-and-toleration/. |
-| tracing.attributes | object | `{}` | Resource attributes to identify the polaris service among other tracing sources. See https://opentelemetry.io/docs/reference/specification/resource/semantic_conventions/#service. If left empty, traces will be attached to a service named "Apache Polaris"; to change this, provide a service.name attribute here. |
-| tracing.enabled | bool | `false` | Specifies whether tracing for the polaris server should be enabled. |
-| tracing.endpoint | string | `"http://otlp-collector:4317"` | The collector endpoint URL to connect to (required). The endpoint URL must have either the http:// or the https:// scheme. The collector must talk the OpenTelemetry protocol (OTLP) and the port must be its gRPC port (by default 4317). See https://quarkus.io/guides/opentelemetry for more information. |
-| tracing.sample | string | `"1.0d"` | Which requests should be sampled. Valid values are: "all", "none", or a ratio between 0.0 and "1.0d" (inclusive). E.g. "0.5d" means that 50% of the requests will be sampled. Note: avoid entering numbers here, always prefer a string representation of the ratio. |
+| tracing.otel | object | `{"attributes":{},"enabled":false,"endpoint":"http://otlp-collector:4317","sample":"1.0d"}` | OpenTelemetry configuration. |
+| tracing.otel.attributes | object | `{}` | Resource attributes to identify the polaris service among other tracing sources. See https://opentelemetry.io/docs/reference/specification/resource/semantic_conventions/#service. If left empty, traces will be attached to a service named "Apache Polaris"; to change this, provide a service.name attribute here. |
+| tracing.otel.enabled | bool | `false` | Specifies whether tracing for the Apache Polaris server should be enabled. When this is enabled, then an OpenTelemetry collector endpoint must be configured. |
+| tracing.otel.endpoint | string | `"http://otlp-collector:4317"` | The collector endpoint URL to connect to (required). The endpoint URL must have either the http:// or the https:// scheme. The collector must talk the OpenTelemetry protocol (OTLP) and the port must be its gRPC port (by default 4317). See https://quarkus.io/guides/opentelemetry for more information. |
+| tracing.otel.sample | string | `"1.0d"` | Which requests should be sampled. Valid values are: "all", "none", or a ratio between 0.0 and "1.0d" (inclusive). E.g. "0.5d" means that 50% of the requests will be sampled. Note: avoid entering numbers here, always prefer a string representation of the ratio. |
+| tracing.requestId | object | `{"headerName":"X-Request-ID"}` | Configuration for the request ID filter. |
+| tracing.requestId.headerName | string | `"X-Request-ID"` | The name of the header that contains the request ID. |

helm/polaris/templates/configmap.yaml

@@ -165,28 +165,31 @@ data:
     {{- range $k, $v := $categories -}}
     {{- $_ = set $map (printf "quarkus.log.category.\"%s\".level" $k) $v -}}
     {{- end -}}
-    {{- $_ = set $map "polaris.log.request-id-header-name" .Values.logging.requestIdHeaderName -}}
     {{- $mdc := dict -}}
     {{- list .Values.logging.mdc "" $mdc | include "polaris.mergeConfigTree" -}}
     {{- range $k, $v := $mdc -}}
     {{- $_ = set $map (printf "polaris.log.mdc.\"%s\"" $k) $v -}}
     {{- end -}}
 
     {{- /* Telemetry */ -}}
-    {{- if .Values.tracing.enabled -}}
-    {{- $_ = set $map "quarkus.otel.exporter.otlp.endpoint" .Values.tracing.endpoint -}}
-    {{- if .Values.tracing.attributes -}}
+    {{- /* TODO remove deprecated configs, keep only tracing.otel.* */ -}}
+    {{- if (or .Values.tracing.enabled .Values.tracing.otel.enabled) -}}
+    {{- $_ = set $map "quarkus.otel.exporter.otlp.endpoint" (coalesce .Values.tracing.endpoint .Values.tracing.otel.endpoint) -}}
     {{- $attributes := dict -}}
+    {{- if .Values.tracing.otel.attributes -}}
+    {{- list .Values.tracing.otel.attributes "" $attributes | include "polaris.mergeConfigTree" -}}
+    {{- end -}}
+    {{- if .Values.tracing.attributes -}}
     {{- list .Values.tracing.attributes "" $attributes | include "polaris.mergeConfigTree" -}}
+    {{- end -}}
     {{- $i := 0 -}}
     {{- range $k, $v := $attributes -}}
     {{- $_ = set $map (printf "quarkus.otel.resource.attributes[%d]" $i) (printf "%s=%s" $k $v) -}}
     {{- $i = add1 $i -}}
     {{- end -}}
-    {{- end -}}
-    {{- if .Values.tracing.sample -}}
-    {{- $sample := toString .Values.tracing.sample -}}
-    {{ if eq $sample "all" -}}
+    {{- if (or .Values.tracing.sample .Values.tracing.otel.sample) -}}
+    {{- $sample := toString (coalesce .Values.tracing.sample .Values.tracing.otel.sample) -}}
+    {{- if eq $sample "all" -}}
     {{- $_ = set $map "quarkus.otel.traces.sampler" "parentbased_always_on" -}}
     {{- else if eq $sample "none" -}}
     {{- $_ = set $map "quarkus.otel.traces.sampler" "always_off" -}}
@@ -198,6 +201,7 @@ data:
     {{- else -}}
     {{- $_ = set $map "quarkus.otel.sdk.disabled" true -}}
     {{- end -}}
+    {{- $_ = set $map "polaris.tracing.request-id.header-name" (coalesce .Values.logging.requestIdHeaderName .Values.tracing.requestId.headerName) -}}
 
     {{- /* Metrics */ -}}
     {{- if .Values.metrics.enabled -}}

helm/polaris/tests/configmap_test.yaml

@@ -292,43 +292,88 @@ tests:
       - matchRegex: { path: 'data["application.properties"]', pattern: "polaris.log.mdc.\"org.acme\"=foo" }
       - matchRegex: { path: 'data["application.properties"]', pattern: "polaris.log.mdc.\"org.acme.service\"=foo" }
 
-  - it: should include telemetry configuration
+  - it: should include deprecated telemetry configuration
     set:
       tracing: { enabled: true, endpoint: http://custom:4317, attributes: { service.name: custom, foo: bar } }
     asserts:
       - matchRegex: { path: 'data["application.properties"]', pattern: "quarkus.otel.exporter.otlp.endpoint=http://custom:4317" }
       - matchRegex: { path: 'data["application.properties"]', pattern: "quarkus.otel.resource.attributes\\[\\d\\]=service.name=custom" }
       - matchRegex: { path: 'data["application.properties"]', pattern: "quarkus.otel.resource.attributes\\[\\d\\]=foo=bar" }
 
-  - it: should include set sample rate numeric
+  - it: should include telemetry configuration
+    set:
+      tracing.otel: { enabled: true, endpoint: http://custom:4317, attributes: { service.name: custom, foo: bar } }
+    asserts:
+      - matchRegex: { path: 'data["application.properties"]', pattern: "quarkus.otel.exporter.otlp.endpoint=http://custom:4317" }
+      - matchRegex: { path: 'data["application.properties"]', pattern: "quarkus.otel.resource.attributes\\[\\d\\]=service.name=custom" }
+      - matchRegex: { path: 'data["application.properties"]', pattern: "quarkus.otel.resource.attributes\\[\\d\\]=foo=bar" }
+
+  - it: should set sample rate numeric (deprecated)
     set:
       tracing: { enabled: true, sample: "0.123" }
     asserts:
       - matchRegex: { path: 'data["application.properties"]', pattern: "quarkus.otel.traces.sampler=parentbased_traceidratio" }
       - matchRegex: { path: 'data["application.properties"]', pattern: "quarkus.otel.traces.sampler.arg=0.123" }
 
-  - it: should include set sample rate "all"
+  - it: should set sample rate numeric
+    set:
+      tracing.otel: { enabled: true, sample: "0.123" }
+    asserts:
+      - matchRegex: { path: 'data["application.properties"]', pattern: "quarkus.otel.traces.sampler=parentbased_traceidratio" }
+      - matchRegex: { path: 'data["application.properties"]', pattern: "quarkus.otel.traces.sampler.arg=0.123" }
+
+  - it: should set sample rate "all" (deprecated)
     set:
       tracing: { enabled: true, sample: "all" }
     asserts:
       - matchRegex: { path: 'data["application.properties"]', pattern: "quarkus.otel.traces.sampler=parentbased_always_on" }
 
-  - it: should include set sample rate "none"
+  - it: should set sample rate "all"
+    set:
+      tracing.otel: { enabled: true, sample: "all" }
+    asserts:
+      - matchRegex: { path: 'data["application.properties"]', pattern: "quarkus.otel.traces.sampler=parentbased_always_on" }
+
+  - it: should set sample rate "none" (deprecated)
     set:
       tracing: { enabled: true, sample: "none" }
     asserts:
       - matchRegex: { path: 'data["application.properties"]', pattern: "quarkus.otel.traces.sampler=always_off" }
 
+  - it: should set sample rate "none"
+    set:
+      tracing.otel: { enabled: true, sample: "none" }
+    asserts:
+      - matchRegex: { path: 'data["application.properties"]', pattern: "quarkus.otel.traces.sampler=always_off" }
+
   - it: should disable tracing by default
     asserts:
       - matchRegex: { path: 'data["application.properties"]', pattern: "quarkus.otel.sdk.disabled=true" }
 
-  - it: should disable tracing
+  - it: should disable tracing (deprecated)
     set:
       tracing: { enabled: false }
     asserts:
       - matchRegex: { path: 'data["application.properties"]', pattern: "quarkus.otel.sdk.disabled=true" }
 
+  - it: should disable tracing
+    set:
+      tracing.otel: { enabled: false }
+    asserts:
+      - matchRegex: { path: 'data["application.properties"]', pattern: "quarkus.otel.sdk.disabled=true" }
+
+  - it: should including request ID header name (deprecated)
+    set:
+      logging.requestIdHeaderName: X-Custom-Request-ID
+    asserts:
+      - matchRegex: { path: 'data["application.properties"]', pattern: "polaris.tracing.request-id.header-name=X-Custom-Request-ID" }
+
+  - it: should including request ID header name
+    set:
+      tracing.requestId: { headerName: X-Custom-Request-ID }
+    asserts:
+      - matchRegex: { path: 'data["application.properties"]', pattern: "polaris.tracing.request-id.header-name=X-Custom-Request-ID" }
+
   - it: should include custom metrics
     set:
       metrics: { enabled: true, tags: { app: custom, foo: bar } }

helm/polaris/values.yaml

@@ -392,25 +392,31 @@ extraInitContainers: []
   #   command: ['sh', '-c', 'echo "hello world"']
 
 tracing:
-  # -- Specifies whether tracing for the polaris server should be enabled.
-  enabled: false
-  # -- The collector endpoint URL to connect to (required).
-  # The endpoint URL must have either the http:// or the https:// scheme.
-  # The collector must talk the OpenTelemetry protocol (OTLP) and the port must be its gRPC port (by default 4317).
-  # See https://quarkus.io/guides/opentelemetry for more information.
-  endpoint: "http://otlp-collector:4317"
-  # -- Which requests should be sampled. Valid values are: "all", "none", or a ratio between 0.0 and
-  # "1.0d" (inclusive). E.g. "0.5d" means that 50% of the requests will be sampled.
-  # Note: avoid entering numbers here, always prefer a string representation of the ratio.
-  sample: "1.0d"
-  # -- Resource attributes to identify the polaris service among other tracing sources.
-  # See https://opentelemetry.io/docs/reference/specification/resource/semantic_conventions/#service.
-  # If left empty, traces will be attached to a service named "Apache Polaris"; to change this,
-  # provide a service.name attribute here.
-  attributes:
-    {}
-    # service.name: my-polaris
-
+  # -- OpenTelemetry configuration.
+  otel:
+    # -- Specifies whether tracing for the Apache Polaris server should be enabled. When this is enabled,
+    # then an OpenTelemetry collector endpoint must be configured.
+    enabled: false
+    # -- The collector endpoint URL to connect to (required).
+    # The endpoint URL must have either the http:// or the https:// scheme.
+    # The collector must talk the OpenTelemetry protocol (OTLP) and the port must be its gRPC port (by default 4317).
+    # See https://quarkus.io/guides/opentelemetry for more information.
+    endpoint: "http://otlp-collector:4317"
+    # -- Which requests should be sampled. Valid values are: "all", "none", or a ratio between 0.0 and
+    # "1.0d" (inclusive). E.g. "0.5d" means that 50% of the requests will be sampled.
+    # Note: avoid entering numbers here, always prefer a string representation of the ratio.
+    sample: "1.0d"
+    # -- Resource attributes to identify the polaris service among other tracing sources.
+    # See https://opentelemetry.io/docs/reference/specification/resource/semantic_conventions/#service.
+    # If left empty, traces will be attached to a service named "Apache Polaris"; to change this,
+    # provide a service.name attribute here.
+    attributes:
+      {}
+      # service.name: my-polaris
+  # -- Configuration for the request ID filter.
+  requestId:
+    # -- The name of the header that contains the request ID.
+    headerName: X-Request-ID
 metrics:
   # -- Specifies whether metrics for the polaris server should be enabled.
   enabled: true
@@ -442,8 +448,6 @@ serviceMonitor:
 logging:
   # -- The log level of the root category, which is used as the default log level for all categories.
   level: INFO
-  # -- The header name to use for the request ID.
-  requestIdHeaderName: Polaris-Request-Id
   # -- Configuration for the console appender.
   console:
     # -- Whether to enable the console appender.