XComp commented on code in PR #19444:
URL: https://github.com/apache/flink/pull/19444#discussion_r853878533
##########
flink-runtime/src/main/java/org/apache/flink/runtime/metrics/ReporterSetup.java:
##########
@@ -360,6 +361,7 @@ private static List<ReporterSetup> setupReporters(
return reporterSetups;
}
+ @SuppressWarnings("deprecation")
private static Optional<MetricReporter> loadReporter(
Review Comment:
What about adding a warn log message after the `factorClassName != null` if
block stating that this option is deprecated to make this more visual to the
user through logs? Or do users have to touch their reporter code anyway when
updating Flink? Then, they would see it through the `@deprecated` annotation of
the `@InterceptInstantiationViaReflection` annotation...
##########
flink-end-to-end-tests/flink-metrics-reporter-prometheus-test/src/test/java/org/apache/flink/metrics/prometheus/tests/PrometheusReporterEndToEndITCase.java:
##########
@@ -324,37 +296,28 @@ private static void checkMetricAvailability(final
OkHttpClient client, final Str
static class TestParams {
private final String jarLocationDescription;
private final Consumer<FlinkResourceSetup.FlinkResourceSetupBuilder>
builderSetup;
- private final InstantiationType instantiationType;
private TestParams(
String jarLocationDescription,
- Consumer<FlinkResourceSetup.FlinkResourceSetupBuilder>
builderSetup,
- InstantiationType instantiationType) {
+ Consumer<FlinkResourceSetup.FlinkResourceSetupBuilder>
builderSetup) {
this.jarLocationDescription = jarLocationDescription;
this.builderSetup = builderSetup;
- this.instantiationType = instantiationType;
}
public static TestParams from(
String jarLocationDesription,
Consumer<FlinkResourceSetup.FlinkResourceSetupBuilder>
builderSetup,
InstantiationType instantiationType) {
- return new TestParams(jarLocationDesription, builderSetup,
instantiationType);
+ return new TestParams(jarLocationDesription, builderSetup);
}
public Consumer<FlinkResourceSetup.FlinkResourceSetupBuilder>
getBuilderSetup() {
return builderSetup;
}
- public InstantiationType getInstantiationType() {
- return instantiationType;
- }
-
@Override
public String toString() {
- return jarLocationDescription
- + ", instantiated via "
- + instantiationType.name().toLowerCase();
+ return jarLocationDescription;
}
public enum InstantiationType {
Review Comment:
The `REFLECTION` type is unused. I guess, we could get rid of the enum
entirely now. No need to have this around...
##########
flink-end-to-end-tests/flink-metrics-reporter-prometheus-test/src/test/java/org/apache/flink/metrics/prometheus/tests/PrometheusReporterEndToEndITCase.java:
##########
@@ -324,37 +296,28 @@ private static void checkMetricAvailability(final
OkHttpClient client, final Str
static class TestParams {
private final String jarLocationDescription;
private final Consumer<FlinkResourceSetup.FlinkResourceSetupBuilder>
builderSetup;
- private final InstantiationType instantiationType;
private TestParams(
String jarLocationDescription,
- Consumer<FlinkResourceSetup.FlinkResourceSetupBuilder>
builderSetup,
- InstantiationType instantiationType) {
+ Consumer<FlinkResourceSetup.FlinkResourceSetupBuilder>
builderSetup) {
this.jarLocationDescription = jarLocationDescription;
this.builderSetup = builderSetup;
- this.instantiationType = instantiationType;
}
public static TestParams from(
String jarLocationDesription,
Consumer<FlinkResourceSetup.FlinkResourceSetupBuilder>
builderSetup,
InstantiationType instantiationType) {
Review Comment:
unused
##########
flink-core/src/main/java/org/apache/flink/configuration/MetricOptions.java:
##########
@@ -67,8 +67,8 @@ public class MetricOptions {
+ " any of the names in the list will be
started. Otherwise, all reporters that could be found in"
+ " the configuration will be started.");
- @Documentation.SuffixOption(NAMED_REPORTER_CONFIG_PREFIX)
- @Documentation.Section(value = Documentation.Sections.METRIC_REPORTERS,
position = 1)
+ /** @deprecated use {@link MetricOptions#REPORTER_FACTORY_CLASS} instead.
*/
+ @Deprecated
public static final ConfigOption<String> REPORTER_CLASS =
Review Comment:
[ConfigConstants#METRICS_REPORTER_CLASS_SUFFIX](https://github.com/apache/flink/blob/d834b271366ed508aba327aa94a14bb2b2e47a4c/flink-core/src/main/java/org/apache/flink/configuration/ConfigConstants.java#L1116)
is referring to this member. Just wanted to mention it but I guess it doesn't
add any value...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
