[
https://issues.apache.org/jira/browse/GOBBLIN-2161?focusedWorklogId=936819&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-936819
]
ASF GitHub Bot logged work on GOBBLIN-2161:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 27/Sep/24 14:22
Start Date: 27/Sep/24 14:22
Worklog Time Spent: 10m
Work Description: khandelwal-prateek commented on code in PR #4061:
URL: https://github.com/apache/gobblin/pull/4061#discussion_r1778709820
##########
gobblin-api/src/main/java/org/apache/gobblin/configuration/ConfigurationKeys.java:
##########
@@ -911,6 +911,12 @@ public class ConfigurationKeys {
public static final String METRICS_REPORTING_OPENTELEMETRY_PREFIX =
"metrics.reporting.opentelemetry.";
public static final String METRICS_REPORTING_OPENTELEMETRY_ENABLED =
METRICS_REPORTING_OPENTELEMETRY_PREFIX + "enabled";
+ public static final String
METRICS_REPORTING_OPENTELEMETRY_LOGEXPORTER_ENABLED =
METRICS_REPORTING_OPENTELEMETRY_PREFIX + "logexporter.enabled";
+
+ public static final Boolean
DEFAULT_METRICS_REPORTING_OPENTELEMETRY_LOGEXPORTER_ENABLED = false;
+
+ public static final String
METRICS_REPORTING_OPENTELEMETRY_LOGEXPORTER_CLASSNAME =
METRICS_REPORTING_OPENTELEMETRY_PREFIX + "logexporter.className";
Review Comment:
I understand that it would be useful to make the MetricExporter
instantiation more generic, allowing for any MetricExporter class to be
instantiated. However, since different exporters may require varying
instantiation methods (e.g., constructors vs. static methods), and we also need
to set certain properties which are specific to `OtlpHttpMetricExporter`, so
it's challenging to handle them all in a truly generic way.
One possible solution could be to use a factory pattern, where we have a
separate factory class for each MetricExporter type. Each factory would know
how to properly instantiate its corresponding MetricExporter. However, this
would require significant changes
```
public interface MetricExporterFactory {
MetricExporter create(State state);
}
@Override
protected MetricExporter initializeMetricExporter(State state) {
MetricExporterFactory factory = getMetricExporterFactory(state);
return factory.create(state);
}
```
Or we can update `initializeMetricExporter` method to use reflection to
instantiate the class specified in the configuration, however, since the
instantiation is not common across different metricExporters, supporting
another metricExporter might still require new code to be handled.
```
protected MetricExporter initializeMetricExporter(State state) {
MetricExporter metricExporter;
String exporterClassName =
state.getProp(ConfigurationKeys.METRICS_REPORTING_OPENTELEMETRY_EXPORTER_CLASSNAME,
OtlpHttpMetricExporter.class.getName());
try {
Class<?> clazz = Class.forName(exporterClassName);
Method instanceMethod = clazz.getMethod("instance");
// Invoke the method to get the singleton instance
metricExporter = (MetricExporter) instanceMethod.invoke(null);
} catch (Exception e) {
log.error("Error occurred while instantiating opentelemetry Exporter
class: " + exporterClassName, e);
metricExporter = createDefaultExporter(state);
}
return metricExporter;
}
```
Let me know if the existing way to create LogMetricExporter is fine given
the constraints or you meant something else.
Issue Time Tracking
-------------------
Worklog Id: (was: 936819)
Time Spent: 20m (was: 10m)
> Create a configurable Opentelemetry exporter
> --------------------------------------------
>
> Key: GOBBLIN-2161
> URL: https://issues.apache.org/jira/browse/GOBBLIN-2161
> Project: Apache Gobblin
> Issue Type: Improvement
> Reporter: William Lo
> Priority: Major
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Currently the Opentelemetry Exporter for Otel metrics are forced to use the
> HttpMetricExporter. This makes it less flexible than if we had a configurable
> exporter to support multiple ways of sending metrics, including a
> logging/debug exporter.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
