From 423b8b4274e47a5618bb8eac1b410b7a40b0c398 Mon Sep 17 00:00:00 2001
From: Serge Tupchii <serge.tupchii@protonmail.com>
Date: Wed, 27 Dec 2023 13:55:53 +0200
Subject: [PATCH] fix(emqx_opentelemetry): use converter to convert legacy
 metrics config

HOCON converter is cleaner and safer option,
that will also work for configurations defined in emqx.conf.
Previously used `upgrade_raw_conf/1` is currently not supported in hocon_cli (used in bin/emqx).

Fixes: EMQX-11643
---
 apps/emqx_conf/src/emqx_conf.app.src          |  2 +-
 apps/emqx_conf/src/emqx_conf_schema.erl       |  3 +-
 .../src/emqx_opentelemetry.app.src            |  2 +-
 .../src/emqx_otel_schema.erl                  | 63 +++++++++----------
 .../test/emqx_otel_schema_SUITE.erl           |  3 +-
 changes/ce/fix-12234.en.md                    |  1 +
 6 files changed, 36 insertions(+), 38 deletions(-)
 create mode 100644 changes/ce/fix-12234.en.md

diff --git a/apps/emqx_conf/src/emqx_conf.app.src b/apps/emqx_conf/src/emqx_conf.app.src
index 7f495a3cd..761f36a11 100644
--- a/apps/emqx_conf/src/emqx_conf.app.src
+++ b/apps/emqx_conf/src/emqx_conf.app.src
@@ -1,6 +1,6 @@
 {application, emqx_conf, [
     {description, "EMQX configuration management"},
-    {vsn, "0.1.32"},
+    {vsn, "0.1.33"},
     {registered, []},
     {mod, {emqx_conf_app, []}},
     {applications, [kernel, stdlib, emqx_ctl]},
diff --git a/apps/emqx_conf/src/emqx_conf_schema.erl b/apps/emqx_conf/src/emqx_conf_schema.erl
index eeb0d6de8..8cfa9a8ea 100644
--- a/apps/emqx_conf/src/emqx_conf_schema.erl
+++ b/apps/emqx_conf/src/emqx_conf_schema.erl
@@ -77,8 +77,7 @@
 
 %% Callback to upgrade config after loaded from config file but before validation.
 upgrade_raw_conf(RawConf) ->
-    RawConf1 = emqx_connector_schema:transform_bridges_v1_to_connectors_and_bridges_v2(RawConf),
-    emqx_otel_schema:upgrade_legacy_metrics(RawConf1).
+    emqx_connector_schema:transform_bridges_v1_to_connectors_and_bridges_v2(RawConf).
 
 namespace() -> emqx.
 
diff --git a/apps/emqx_opentelemetry/src/emqx_opentelemetry.app.src b/apps/emqx_opentelemetry/src/emqx_opentelemetry.app.src
index 95249ff85..4cde26326 100644
--- a/apps/emqx_opentelemetry/src/emqx_opentelemetry.app.src
+++ b/apps/emqx_opentelemetry/src/emqx_opentelemetry.app.src
@@ -1,6 +1,6 @@
 {application, emqx_opentelemetry, [
     {description, "OpenTelemetry for EMQX Broker"},
-    {vsn, "0.2.1"},
+    {vsn, "0.2.2"},
     {registered, []},
     {mod, {emqx_otel_app, []}},
     {applications, [
diff --git a/apps/emqx_opentelemetry/src/emqx_otel_schema.erl b/apps/emqx_opentelemetry/src/emqx_otel_schema.erl
index be14a2b29..04d12c8c7 100644
--- a/apps/emqx_opentelemetry/src/emqx_otel_schema.erl
+++ b/apps/emqx_opentelemetry/src/emqx_otel_schema.erl
@@ -24,40 +24,15 @@
     desc/1
 ]).
 
--export([upgrade_legacy_metrics/1]).
-
-%% Compatibility with the previous schema that defined only metric fields
-upgrade_legacy_metrics(RawConf) ->
-    case RawConf of
-        #{<<"opentelemetry">> := Otel} ->
-            Otel1 =
-                case maps:take(<<"enable">>, Otel) of
-                    {MetricsEnable, OtelConf} ->
-                        emqx_utils_maps:deep_put(
-                            [<<"metrics">>, <<"enable">>], OtelConf, MetricsEnable
-                        );
-                    error ->
-                        Otel
-                end,
-            Otel2 =
-                case Otel1 of
-                    #{<<"exporter">> := #{<<"interval">> := Interval} = Exporter} ->
-                        emqx_utils_maps:deep_put(
-                            [<<"metrics">>, <<"interval">>],
-                            Otel1#{<<"exporter">> => maps:remove(<<"interval">>, Exporter)},
-                            Interval
-                        );
-                    _ ->
-                        Otel1
-                end,
-            RawConf#{<<"opentelemetry">> => Otel2};
-        _ ->
-            RawConf
-    end.
-
 namespace() -> opentelemetry.
 
-roots() -> ["opentelemetry"].
+roots() ->
+    [
+        {"opentelemetry",
+            ?HOCON(?R_REF("opentelemetry"), #{
+                converter => fun legacy_metrics_converter/2
+            })}
+    ].
 
 fields("opentelemetry") ->
     [
@@ -259,3 +234,27 @@ desc("otel_metrics") -> ?DESC(otel_metrics);
 desc("otel_traces") -> ?DESC(otel_traces);
 desc("trace_filter") -> ?DESC(trace_filter);
 desc(_) -> undefined.
+
+%% Compatibility with the previous schema that defined only metrics fields
+legacy_metrics_converter(OtelConf, _Opts) when is_map(OtelConf) ->
+    Otel1 =
+        case maps:take(<<"enable">>, OtelConf) of
+            {MetricsEnable, OtelConf1} ->
+                emqx_utils_maps:deep_put(
+                    [<<"metrics">>, <<"enable">>], OtelConf1, MetricsEnable
+                );
+            error ->
+                OtelConf
+        end,
+    case Otel1 of
+        #{<<"exporter">> := #{<<"interval">> := Interval} = Exporter} ->
+            emqx_utils_maps:deep_put(
+                [<<"metrics">>, <<"interval">>],
+                Otel1#{<<"exporter">> => maps:remove(<<"interval">>, Exporter)},
+                Interval
+            );
+        _ ->
+            Otel1
+    end;
+legacy_metrics_converter(Conf, _Opts) ->
+    Conf.
diff --git a/apps/emqx_opentelemetry/test/emqx_otel_schema_SUITE.erl b/apps/emqx_opentelemetry/test/emqx_otel_schema_SUITE.erl
index abc6548f9..22c93e307 100644
--- a/apps/emqx_opentelemetry/test/emqx_otel_schema_SUITE.erl
+++ b/apps/emqx_opentelemetry/test/emqx_otel_schema_SUITE.erl
@@ -22,8 +22,7 @@
 -include_lib("common_test/include/ct.hrl").
 -include_lib("snabbkaffe/include/snabbkaffe.hrl").
 
-%% Backward compatibility suite for `upgrade_raw_conf/1`,
-%% expected callback is `emqx_otel_schema:upgrade_legacy_metrics/1`
+%% Backward compatibility suite for legacy metrics converter
 
 -define(OLD_CONF_ENABLED, <<
     "\n"
diff --git a/changes/ce/fix-12234.en.md b/changes/ce/fix-12234.en.md
new file mode 100644
index 000000000..ac6f105ef
--- /dev/null
+++ b/changes/ce/fix-12234.en.md
@@ -0,0 +1 @@
+Fix old (prior to EMQX 5.4.0) Open Telemetry configuration incompatibility when the config is defined in emqx.conf.