Support struct values for fields in OTEL AccessLogger

[TAOXUY]

Description:
the existing otel logger's request format is static by the body and attributes field(the struct layout is defined at the config time). I am wondering if we can make that dynamic by supporting struct values for fields in OTEL AccessLogger

[soulxu]

cc @htuch

[htuch]

Is the idea to use header formatter variables?

[TAOXUY]

Hi @htuch , possibly extend access logging formatter somehow.

Currently, if the var is set to a dynamic_metadata which is struct, it will be set as a json string but we need to be a struct in OTel accesslogger.

@kyessenov

[htuch]

Can you propose a proto change to make this concrete?

[kyessenov]

I had some offline discussion. @TAOXUY I think you should share the use case for the log. AFAICT it's not a request log, it's closer to resource/audit log, which might require multiple entries per-request, for example. In that case, it's OTLP only needs to provide the client network function and receive the entire entry plus resource dynamically.

[TAOXUY]

Thanks @kyessenov!

Yeah, that's our further ask. Essentially we could generate multiple logs(audit log) per access dynamically so we want to the forerunner extension can fully control the OTEL request generation, instead of using the current static declarative way.

[htuch]

Thanks. I think having a concrete proto change (even if only a sketch in a comment) would be a useful way to clarify what is being proposed.

[kyessenov]

@htuch This might actually be a more important question than this PR. The problem is that Wasm extensions have a choice to do two things:

1. They can import Otel SDK and compile it to Wasm directly, which requires substituting POSIX I/O calls with appropriate Envoy ABI calls.
   1b. They can go the null Wasm way, and skip the second step in 1). This is how istio/proxy Stackdriver extension operates, by importing OpenCensus SDK.
2. They can use Envoy's Otel log sink as the SDK, and create a "foreign function" as an ABI extension to directly emit a log entry from the module, and let Envoy only handle batching and export. This can also be done indirectly by writing some well known filter state or dynamic metadata that is consumed by Otel log sink.

Each approach has merits, so it would be useful to provide some guidance on which approach to take with respect to extensions.

[TAOXUY]

Thanks Kuat!

For

envoy/api/envoy/extensions/access_loggers/open_telemetry/v3/logs_service.proto

Lines 26 to 49 in da09811

	message OpenTelemetryAccessLogConfig {
	// [#comment:TODO(itamarkam): add 'filter_state_objects_to_log' to logs.]
	grpc.v3.CommonGrpcAccessLogConfig common_config = 1 [(validate.rules).message = {required: true}];
	// If specified, Envoy will not generate built-in resource labels
	// like ``log_name``, ``zone_name``, ``cluster_name``, ``node_name``.
	bool disable_builtin_labels = 5;
	// OpenTelemetry `Resource <https://github.com/open-telemetry/opentelemetry-proto/blob/main/opentelemetry/proto/logs/v1/logs.proto#L51>`_
	// attributes are filled with Envoy node info.
	// Example: ``resource_attributes { values { key: "region" value { string_value: "cn-north-7" } } }``.
	opentelemetry.proto.common.v1.KeyValueList resource_attributes = 4;
	// OpenTelemetry `LogResource <https://github.com/open-telemetry/opentelemetry-proto/blob/main/opentelemetry/proto/logs/v1/logs.proto>`_
	// fields, following `Envoy access logging formatting <https://www.envoyproxy.io/docs/envoy/latest/configuration/observability/access_log/usage>`_.
	//
	// See 'body' in the LogResource proto for more details.
	// Example: ``body { string_value: "%PROTOCOL%" }``.
	opentelemetry.proto.common.v1.AnyValue body = 2;
	// See 'attributes' in the LogResource proto for more details.
	// Example: ``attributes { values { key: "user_agent" value { string_value: "%REQ(USER-AGENT)%" } } }``.
	opentelemetry.proto.common.v1.KeyValueList attributes = 3;
	}

I am thinking introducing something like

// When the field is set, the  the OTel AccessLogger will assume
// a request is set in this filterState by precursor extensions
// so it will add this request in the request batches. 
// When this field is used, `resource_attributes`, `body` or `attributes` shouldn't be set.
 string filter_state_for_otel_request_injection =6;

Why we need this?

1. we may have multiple audit logs dynamically generated for single request so this static declarative configuring approach for request generation will no longer be effective.
2. we don't need to worry about Support struct values for fields in OTEL AccessLogger #31148
3. we don't need to worry about access_log: add posibility to omit empty values for opentelemetry access log #30017
4. minor: we potentially set OTel fields(other than Resource Attributes, LogRecord.body, LogRecord.body.attributes) that are not exposed by OTel AccessLogger config.

[htuch]

For #31148 (comment), I feel this speaks to limitation in ProxyWasm and our ABI (CC @mpwarres @PiotrSikora). Why can't we have better ABI-level support for logging/tracing/stats that can approach what is needed for OTLP?

I'm actually super confused on this thread though, it started off as some ask about OT access loggers, and now it's about Wasm ABIs?

[kyessenov]

@htuch The original issue did not have the full context. The logs are being produced by a Wasm extension, and need to be exported via OTLP to a different resource (not the request log, multiple entries per log).

[PiotrSikora]

I'm definitely missing some context, but what's missing in Wasm's proxy_log?

What's the exact ask that would make Wasm work for OTEL AccessLogger?

[kyessenov]

@PiotrSikora Ability to send structured logs (https://github.com/open-telemetry/opentelemetry-proto/blob/main/opentelemetry/proto/logs/v1/logs.proto), meaning key-value attribute, resource attributes, and structured body.

Also, ability to register OTLP for Wasm, and route proxy_log to it.

[github-actions]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

[github-actions]

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.