You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/03/28 15:28:39 UTC

[GitHub] [flink] metaswirl commented on a change in pull request #19234: [FLINK-26851][metrics] Properly model reporter options as ConfigOptions

metaswirl commented on a change in pull request #19234:
URL: https://github.com/apache/flink/pull/19234#discussion_r836552447



##########
File path: docs/content/docs/deployment/metric_reporters.md
##########
@@ -35,18 +35,15 @@ For more information about Flink's metric system go to the [metric system docume
 Metrics can be exposed to an external system by configuring one or several reporters in `conf/flink-conf.yaml`. These
 reporters will be instantiated on each job and task manager when they are started.
 
-- `metrics.reporter.<name>.<config>`: Generic setting `<config>` for the reporter named `<name>`.
-- `metrics.reporter.<name>.class`: The reporter class to use for the reporter named `<name>`.
-- `metrics.reporter.<name>.factory.class`: The reporter factory class to use for the reporter named `<name>`.
-- `metrics.reporter.<name>.interval`: The reporter interval to use for the reporter named `<name>`.
-- `metrics.reporter.<name>.scope.delimiter`: The delimiter to use for the identifier (default value use `metrics.scope.delimiter`) for the reporter named `<name>`.
-- `metrics.reporter.<name>.scope.variables.excludes`: (optional) A semi-colon (;) separate list of variables that should be ignored by tag-based reporters (e.g., Prometheus, InfluxDB). 
-- `metrics.reporter.<name>.scope.variables.additional`: (optional) A comma separated map of variables and their values, which are separated by a colon (:). These mappings are added to the variable map by tag-based reporters (e.g. Prometheux, InfluxDB). 
-- `metrics.reporters`: (optional) A comma-separated include list of reporter names. By default all configured reporters will be used.
+Below is a list of parameters that are generally applicable to all reporters.
+Reporters may additionally offer implementation-specific parameters, which are documented in the respective reporter's section.
+
+{{< include_reporter_config "layouts/shortcodes/generated/metric_reporters_section.html" >}}
+
+All properties are configured by setting `metrics.reporter.<reporter_name>.<property>` in the configuration.

Review comment:
       ```suggestion
   Below is a list of parameters that are generally applicable to all reporters. All properties are configured by setting `metrics.reporter.<reporter_name>.<property>` in the configuration. Reporters may additionally offer implementation-specific parameters, which are documented in the respective reporter's section. 
   
   {{< include_reporter_config "layouts/shortcodes/generated/metric_reporters_section.html" >}}
   ```

##########
File path: docs/content.zh/docs/deployment/metric_reporters.md
##########
@@ -35,18 +35,15 @@ For more information about Flink's metric system go to the [metric system docume
 Metrics can be exposed to an external system by configuring one or several reporters in `conf/flink-conf.yaml`. These
 reporters will be instantiated on each job and task manager when they are started.
 
-- `metrics.reporter.<name>.<config>`: Generic setting `<config>` for the reporter named `<name>`.
-- `metrics.reporter.<name>.class`: The reporter class to use for the reporter named `<name>`.
-- `metrics.reporter.<name>.factory.class`: The reporter factory class to use for the reporter named `<name>`.
-- `metrics.reporter.<name>.interval`: The reporter interval to use for the reporter named `<name>`.
-- `metrics.reporter.<name>.scope.delimiter`: The delimiter to use for the identifier (default value use `metrics.scope.delimiter`) for the reporter named `<name>`.
-- `metrics.reporter.<name>.scope.variables.excludes`: (optional) A semi-colon (;) separate list of variables that should be ignored by tag-based reporters (e.g., Prometheus, InfluxDB). 
-- `metrics.reporters`: (optional) A comma-separated include list of reporter names. By default all configured reporters will be used.
-- `metrics.reporter.<name>.scope.variables.additional`: (optional) A comma separated map of variables and their values, which are separated by a colon (:). These mappings are added to the variable map by tag-based reporters (e.g. Prometheux, InfluxDB).
+Below is a list of parameters that are generally applicable to all reporters.
+Reporters may additionally offer implementation-specific parameters, which are documented in the respective reporter's section.
+
+{{< include_reporter_config "layouts/shortcodes/generated/metric_reporters_section.html" >}}
+
+All properties are configured by setting `metrics.reporter.<reporter_name>.<property>` in the configuration.

Review comment:
       ```suggestion
   Below is a list of parameters that are generally applicable to all reporters. All properties are configured by setting `metrics.reporter.<reporter_name>.<property>` in the configuration. Reporters may additionally offer implementation-specific parameters, which are documented in the respective reporter's section. 
   
   {{< include_reporter_config "layouts/shortcodes/generated/metric_reporters_section.html" >}}
   ```

##########
File path: flink-docs/src/test/java/org/apache/flink/docs/configuration/ConfigOptionsDocsCompletenessITCase.java
##########
@@ -81,59 +80,54 @@ public void testCompleteness() throws IOException, ClassNotFoundException {
                             final List<ExistingOption> existingOptions = entry.getValue();
                             final List<ExistingOption> consolidated;
 
-                            if (existingOptions.stream()
-                                    .allMatch(option -> option.isSuffixOption)) {
-                                consolidated = existingOptions;
-                            } else {
-                                Optional<ExistingOption> deduped =
-                                        existingOptions.stream()
-                                                .reduce(
-                                                        (option1, option2) -> {
-                                                            if (option1.equals(option2)) {
-                                                                // we allow multiple instances of
-                                                                // ConfigOptions with the same key
-                                                                // if they are identical
-                                                                return option1;
+                            Optional<ExistingOption> deduped =
+                                    existingOptions.stream()
+                                            .reduce(
+                                                    (option1, option2) -> {
+                                                        if (option1.equals(option2)) {
+                                                            // we allow multiple instances of
+                                                            // ConfigOptions with the same key
+                                                            // if they are identical
+                                                            return option1;
+                                                        } else {
+                                                            // found a ConfigOption pair with
+                                                            // the same key that aren't equal
+                                                            // we fail here outright as this is
+                                                            // not a documentation-completeness
+                                                            // problem
+                                                            if (!option1.defaultValue.equals(
+                                                                    option2.defaultValue)) {
+                                                                String errorMessage =
+                                                                        String.format(
+                                                                                "Ambiguous option %s due to distinct default values (%s (in %s) vs %s (in %s)).",

Review comment:
       ```suggestion
                                                                                   "Ambiguous option %s due to distinct default values: %s (in %s) vs %s (in %s)",
   ```

##########
File path: docs/layouts/shortcodes/generated/metric_configuration.html
##########
@@ -62,12 +62,36 @@
             <td>String</td>
             <td>The reporter class to use for the reporter named &lt;name&gt;.</td>
         </tr>
+        <tr>
+            <td><h5>metrics.reporter.&lt;name&gt;.factory.class</h5></td>
+            <td style="word-wrap: break-word;">(none)</td>
+            <td>String</td>
+            <td>The reporter factory class to use for the reporter named &lt;name&gt;.</td>
+        </tr>
         <tr>
             <td><h5>metrics.reporter.&lt;name&gt;.interval</h5></td>
             <td style="word-wrap: break-word;">10 s</td>
             <td>Duration</td>
             <td>The reporter interval to use for the reporter named &lt;name&gt;.</td>
         </tr>
+        <tr>
+            <td><h5>metrics.reporter.&lt;name&gt;.scope.delimiter</h5></td>
+            <td style="word-wrap: break-word;">"."</td>
+            <td>String</td>
+            <td>The delimiter used to assemble the metric identifier for the reporter named &lt;name&gt;.</td>
+        </tr>
+        <tr>
+            <td><h5>metrics.reporter.&lt;name&gt;.scope.variables.additional</h5></td>
+            <td style="word-wrap: break-word;"></td>

Review comment:
       ```suggestion
               <td style="word-wrap: break-word;">(none)</td>
   ```




-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org