EMQX-10534 integrate otel log handler #11921

SergeTupchiy · 2023-11-09T18:54:11Z

Closes: https://emqx.atlassian.net/browse/EMQX-10534

Documentation follow-up ticket: https://emqx.atlassian.net/browse/EMQX-11385
Tests follow-up ticket: https://emqx.atlassian.net/browse/EMQX-11432

Summary

`🤖 Generated by Copilot at 0cfb424`

This pull request adds support for external log handlers using the OpenTelemetry protocol in EMQ X. It updates the dependencies, the configuration schema, and the application version in mix.exs and emqx_opentelemetry.app.src. It adds a new behaviour emqx_external_log_handler and a new module emqx_otel_log_handler that implements it. It modifies emqx_config_logger to enable the external log handlers. It also adds a new schema otel_log_handler and otel_exporter to define the configuration options for the OpenTelemetry log handler and exporter.

PR Checklist

Please convert it to a draft if any of the following conditions are not met. Reviewers may skip over until all the items are checked:

Added tests for the changes
Added property-based tests for code which performs user input validation
Changed lines covered in coverage report
Change log has been added to changes/(ce|ee)/(feat|perf|fix|breaking)-<PR-id>.en.md files
For internal contributor: there is a jira ticket to track this change
Created PR to emqx-docs if documentation update is required, or link to a follow-up jira ticket
Schema changes are backward compatible

Checklist for CI (.github/workflows) changes

If changed package build workflow, pass this action (manual trigger)
Change log has been added to changes/ dir for user-facing artifacts update

SergeTupchiy · 2023-11-13T15:30:52Z

apps/emqx/src/bhvrs/emqx_external_log_handler.erl

+    lists:flatmap(fun(M) -> M:tr_handlers(Conf) end, tr_modules()).
+
+tr_modules() ->
+    [emqx_otel_log_handler].


This probably should be better decoupled from the main emqx app, but I tried to keep it simple for now.

can we install the handler at runtime? i.e. in emqx_opentelemetry application

@zmstone, yes, but it would mean that before emqx_opentelemetry app is started, logs won't be sent to the external otel collector/backend. Unless we start emqx_opentelemetry earlier than any other app.

BTW, current otel_log_handler implementation anyway doesn't allow to start a handler at early stage (when kernel logger starts). I think we have two options for now:

change otel_log_handler to start under kernel's logger_sup, start accepting log event writes to the table but postpone 'full init' before kernel is fully started.
Advantages: the handler can work similarly to native log handlers with no log event losses.
Drawbacks: - have to rely on some kernel internals (logger_sup),
- more complicated log_handler design is required

keep otel_log_handler as is and accept the fact that it can't be initialized at early boot stage, meaning some logs will always be missing (probably up to the point when emqx_conf app initiates).

Note: my previous comment is not absolutely correct. There was actually a deadlock trying to start ssl app (opentelemetry_exporter dependency) in adding_handler/1 logger cb. I'm fixing that now in opentelemetry lib, so starting a handler under kernel logger_sup shouldn't be required.

I don't think it's so critical to have the boot logs sent to otel ?

Moved everything to emqx_opentelemetry app

apps/emqx_conf/src/emqx_conf_schema.erl

zmstone · 2023-11-13T18:45:11Z

apps/emqx_opentelemetry/src/emqx_otel_schema.erl

@@ -29,18 +29,18 @@ roots() -> ["opentelemetry"].

 fields("opentelemetry") ->
    [
-        {exporter,
+        {"exporter",


keep using atoms for filed names

Q: why is it better to use atoms for these?

Changed back to atoms

sorry for missing this reply.

I think we should try to use atoms for field names everywhere so we are one step closer to avoid generating atoms.

SergeTupchiy · 2023-11-17T14:51:36Z

NOTE:
Thanks to upgrade_raw_conf/1, the new feature is backward compatible with cluster.hocon/emqx.conf that define metrics config under opentelmetry root key.
However, it requires Dashboard UI update to conform with the new schema.

rel/i18n/emqx_otel_schema.hocon

ieQu1 · 2023-11-21T19:53:14Z

rel/i18n/emqx_otel_schema.hocon

+exporter.label: "Exporter"
+
+max_queue_size.desc:
+"""The maximum queue size. After the size is reached Open Telemetry signals are dropped."""


Q: What is the strategy for dropping the signals? Does it drop the new signals until the queue is drained, or discards the queued data as well?

It discards new signals. No need to discard the queued data, since olp mechanism generally ensures that the 'queue' stops growing once the limit is reached. The 'queue' is actually not a real queue data structure, but an ETS table.
We keep using 'max_queue_size' because it's a term used in OpenTelemetry specification and our ETS -based implementation doesn't violate the spec.
Each insert to ETS table is done after decrementing and getting the value of the atomic counter. Once counter is decremented to 0 (it starts with max_queue_size), the new events are drooped and the processor (gen_statem process) is notified to start a next exporting cycle.
The actual implementation is slightly more complex since there are two ETS tables and they are reversed periodically, so that when the processor starts exporting, another table becomes available for inserts.
https://github.com/emqx/opentelemetry-erlang/blob/main/apps/opentelemetry/src/otel_batch_olp.erl#L178

Thanks. Perhaps it's worth mentioning in the documentation?

rel/i18n/emqx_otel_schema.hocon

apps/emqx_opentelemetry/src/emqx_otel_schema.erl

zmstone · 2023-11-21T20:10:53Z

apps/emqx_opentelemetry/src/emqx_otel_schema.erl

+            ?HOCON(
+                ?R_REF(emqx_schema, "ssl_client_opts"),
+                #{
+                    default => #{<<"enable">> => false},


it seems ssl enable/disable is derived from the endpoint url scheme.
maybe create a new type here and call emqx_schema:common_ssl_opts_schema (export it) for the fields.

i.e. no need for enable field.

Fixed slightly differently, following similar cases in other schemas, e.g.: https://github.com/emqx/emqx/blob/master/apps/emqx_ldap/src/emqx_ldap.erl#L100

SergeTupchiy changed the base branch from master to release-54 November 9, 2023 18:54

SergeTupchiy changed the title ~~Emqx 10534 integrate otel log handler~~ EMQX-10534 integrate otel log handler Nov 9, 2023

SergeTupchiy force-pushed the EMQX-10534-integrate-otel-log-handler branch 2 times, most recently from 8f33482 to 24a327a Compare November 13, 2023 15:28

SergeTupchiy commented Nov 13, 2023

View reviewed changes

apps/emqx_conf/src/emqx_conf_schema.erl Outdated Show resolved Hide resolved

SergeTupchiy force-pushed the EMQX-10534-integrate-otel-log-handler branch from 24a327a to 4b8e9ab Compare November 13, 2023 16:23

SergeTupchiy marked this pull request as ready for review November 13, 2023 17:17

SergeTupchiy requested review from lafirest and a team as code owners November 13, 2023 17:17

zmstone reviewed Nov 13, 2023

View reviewed changes

SergeTupchiy force-pushed the EMQX-10534-integrate-otel-log-handler branch from 4b8e9ab to f3d687c Compare November 17, 2023 14:31

ieQu1 reviewed Nov 21, 2023

View reviewed changes

rel/i18n/emqx_otel_schema.hocon Outdated Show resolved Hide resolved

ieQu1 reviewed Nov 21, 2023

View reviewed changes

rel/i18n/emqx_otel_schema.hocon Outdated Show resolved Hide resolved

ieQu1 reviewed Nov 21, 2023

View reviewed changes

rel/i18n/emqx_otel_schema.hocon Outdated Show resolved Hide resolved

ieQu1 reviewed Nov 21, 2023

View reviewed changes

zmstone reviewed Nov 21, 2023

View reviewed changes

feat: integrate OpenTelemetry log handler

d9f95cd

SergeTupchiy force-pushed the EMQX-10534-integrate-otel-log-handler branch from f3d687c to d9f95cd Compare November 22, 2023 11:59

ieQu1 approved these changes Nov 22, 2023

View reviewed changes

zmstone approved these changes Nov 22, 2023

View reviewed changes

SergeTupchiy merged commit 275cf05 into emqx:release-54 Nov 22, 2023
162 checks passed

This was referenced Dec 22, 2023

emqx 5.4.0 Homebrew/homebrew-core#158053

Merged

emqx: use otp26 Homebrew/homebrew-core#158059

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EMQX-10534 integrate otel log handler #11921

EMQX-10534 integrate otel log handler #11921

SergeTupchiy commented Nov 9, 2023 •

edited

SergeTupchiy Nov 13, 2023

zmstone Nov 13, 2023

SergeTupchiy Nov 14, 2023

SergeTupchiy Nov 14, 2023

SergeTupchiy Nov 14, 2023

zmstone Nov 16, 2023

SergeTupchiy Nov 17, 2023

zmstone Nov 13, 2023

thalesmg Nov 13, 2023

SergeTupchiy Nov 17, 2023

zmstone Nov 22, 2023

SergeTupchiy commented Nov 17, 2023

ieQu1 Nov 21, 2023

SergeTupchiy Nov 22, 2023

ieQu1 Nov 22, 2023

zmstone Nov 21, 2023

SergeTupchiy Nov 22, 2023

EMQX-10534 integrate otel log handler #11921

EMQX-10534 integrate otel log handler #11921

Conversation

SergeTupchiy commented Nov 9, 2023 • edited

Summary

🤖 Generated by Copilot at 0cfb424

PR Checklist

Checklist for CI (.github/workflows) changes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SergeTupchiy commented Nov 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SergeTupchiy commented Nov 9, 2023 •

edited

`🤖 Generated by Copilot at 0cfb424`