Metric naming & application name as tag
DonDebonair opened this issue · 18 comments
Hi folks,
At KLM we're using PrometheusSink with great success. What we're missing though, is the ability to filter on application name. Currently, the metric names themselves are constructed using a lot of variables, including the application (attempt id), so we get metric names like:
application_1534430873940_0001_driver_UserSessionProcessor_StreamingMetrics_streaming_lastCompletedBatch_totalDelay
This makes it hard to aggregate metrics for a specific Spark application over time, across redeployments. I think it makes more sense to remove the application id from the metric name, so the metric name doesn't change when you deploy a new version. If that is not desired in your opinion, the next best thing would be to at least add the application name (UserSessionProcessor in the example) as label, so it becomes slightly easier to aggregate on.
What are your thoughts? If you agree with one or both solutions, I can make a PR for this.
Hi @dandydev , have you tried setting spark.metrics.namespace for your spark job? e.g spark.metrics.namespace=${spark.app.name}
I was able to verify that it sort of works for us. If we set the spark.metrics.namespace configuration setting to the actual application name, the result is that the job label now holds the application name. The metric name is still the same however.
In practice, by using your suggestion we can now use queries like this in Grafana: {__name__=~".*waitingBatches", exported_job="UserSessionProcessor"}, which makes the resulting graphs generic at least (we used to have to update the graph query after each deployment because the applicationId - and thus the metric name - changed).
I still see a problem however, because after each deployment, the metric name still changes, and thus a new time series is created in Prometheus. This is not desirable, and leads to having multiple lines for what should be the same metric in 1 Grafana graph (based on the aforementioned query).
If you look at the Prometheus best practices, specifically the documentation about the data model and the part about Metric and label naming, you'll see that Prometheus advocates using more generic metric names:
The metric name specifies the general feature of a system that is measured
When I translate this to Spark metrics, I'd expect the metric names to be something like: spark_streaming_lastCompletedBatch_totalDelay (as opposed to the metric name shared in my original issue). You could then differentiate between the totalDelay of different applications (eg. UserSessionProcessor) and different deployments/runs (eg. application_1534430873940_0001) by providing those as labels in each metric. A metric would look like this:
spark_streaming_lastCompletedBatch_totalDelay{job="UserSessionProcessor",instance="application_1534430873940_0001"}
The result would be, that for spark_streaming_lastCompletedBatch_totalDelay only one time series is created in Prometheus, regardless of what application and/or deployment/run we're looking at. This would result in a nice continuous line in Grafana, while you still look at individual apps and/or runs by filtering on labels. But now you also have the possibility to aggregate across those dimensions for a metric.
I've looked at the source code of PrometheusSink, but found it hard to navigate frankly, also due to the mixture of Java and Scala. I tried to hack a solution together that would result in the above suggestion, but didn't get it working so far.
I think it would be great to have a general usable PrometheusSink for Spark, but in its current state this is not it.
What are your thoughts on this?
I had the same issue, I worked around it using relabeling in prometheus (regexps I posted use default format that sink is providing), but it would be better to have this come in in desirable format - so I am also looking for other solution then this.
metric_relabel_configs:
- source_labels: [__name__]
regex: '^application_(\d{10,})_(\d{4})_driver_(.*)'
replacement: $1
target_label: spark_app_name
- source_labels: [__name__]
regex: '^application_(\d{10,})_(\d{4})_driver_(.*)'
replacement: $2
target_label: spark_app_instance
- source_labels: [__name__]
regex: '^application_(\d{10,})_(\d{4})_driver_(.*)'
replacement: $3
target_label: __name__
@zefir6 that's also a good solution for getting the right labels. But yeah, this should be built into the Sink itself.
Question: going by the regexes you posted, $1 is not the app name right? That would be the app id, and $2 would be the attempt id. Or am I overlooking something?
@dandydev the metrics name is constructed by Spark itself, it's out of scope of the Sink.
(see:
https://github.com/apache/spark/blob/master/streaming/src/main/scala/org/apache/spark/streaming/StreamingSource.scala#L27
https://github.com/apache/spark/blob/master/streaming/src/main/scala/org/apache/spark/streaming/StreamingSource.scala#L33
https://github.com/apache/spark/blob/master/streaming/src/main/scala/org/apache/spark/streaming/StreamingSource.scala#L84
https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/metrics/MetricsSystem.scala#L132-L148
as an example)
The Sink currently is using DropwizzardExports to convert the Spark metrics to the format expected by Prometheus.
The Sink could be changed such as it reads an regular expression from config that would be applied on metrics names to transform them prior pushing them over the wire to Pushgateway/Prometheus.
This would be very limited only to metrics names.
Now the question how different is this from what Prometheus relabelling already provides and if it buys us anything. IMHO this would not be as feature rich and flexible as Prometheus relabelling already is thus I'm inclined to say that Prometheus relabelling is the solution for this use case.
Is there anything that I've missed that should be taken into consideration whether it worth adding the metrics name "relabelling" to the Sink itself?
@stoader
In out organization we have limited ability to relabel on prometheus, so it would be useful to be able to deliver metrics in a form that is already "clean".
Also - I think it would be more useful to be able to set labels for metric using sink then anything else (it's not possible right now, am I correct)?
@dandydev @zefir6 I added some limited regular expression based metric name processing to the Sink.
There are two new config settings that you need to add to your metrics.properties file:
metrics-name-capture-regex and metrics-name-replacement. (See https://github.com/banzaicloud/spark-metrics/blob/master/PrometheusSink.md#how-to-enable-prometheussink-in-spark for details).
Also the application instance (spark.app.id) is now passed through in a separate label called "instance" to Prometheus.
These changes have been added to version 2.3-1.1.0.
Please try it out and let me know how it works for you.
Wow @stoader many thanks for quick reaction. One question - this only allows us to change the metric name, right? Not apply additional labels? I am struggling to apply labels on the spark side (our central prometheus requires specific label for metric to be scraped) and we weren't able to use this function from within spark app:
private static void appendLabels(StringBuilder sb, List<String> labels, List<String> values) {
sb.append('{');
for (int i = 0; i < labels.size(); ++i) {
if (i != 0) {
sb.append(',');
}
sb.append(labels.get(i)).append("=\"");
appendEscapedLabelValue(sb, values.get(i));
sb.append('\"');
}
sb.append('}');
}
Would it be possible to add that to the conf properties somehow? So the metric has specific labels you select? That would make sink absolutely perfect solution.
@stoader thanks, we're going to try this out!
edit: it would still be great to add the app-name as label as well, so we don't have to resort to the whole namespace "hack"
@zefir6 yes this only allows changing the metric name using a regular expression.
The appendLabels is for adding the labels of the metrics published by Spark.
Can you describe your use case regarding additional labels coming from config in more detail?
@dandydev I'll add an app-name label as well in a similar way as instance
@stoader Well, in our specific case we need to provide specific label that specifies owner of the app/metric, for it to be scraped by prometheus. If the label is not set, the metric is ignored.
For other use cases:
scripting the given applications run from the level they are submitted (so, we set application name label)
adding specific owner as a label
adding specific team name as a label
It has a lot of usages, and will allow team that writes the applications and runs them, to go a lot of useful graphing based on that in grafana afterwards.
Currently this is how it looks in the pushgateway:
sparkapptest1_driver_LiveListenerBus_listenerProcessingTime_org_apache_spark_status_AppStatusListener{instance="",job="sparkapptest1",role="driver",quantile="0.99"} 0.188832081
If I was able to add more labels here, it would be great complete solution.
@zefir6 please give it a try to the latest version. You can specify the list of labels to be published to Prometheus with each metrics on top of the default ones in your metrics.properties configuration file:
*.sink.<your-sink>.labels=label1=value1,label2=value2
Please let me know how it goes.
@stoader Hey, still testing this - we had some other issues unrelated to this that are delaying, will let you know ASAP when we get it working. Also - your responsivness is greatly appreciated!
@stoader One more thing, in the readme you mention:
Supported only in version 2.3-1.1.0 and above
But then in the example there is:
--packages com.banzaicloud:spark-metrics_2.11:2.3-1.0.1,io.prometheus:simpleclient:0.3.0,io.prometheus:simpleclient_dropwizard:0.3.0,io.prometheus:sim
So which one is it?
2.3-1.0.1 or 2.3-1.1.0
And is there a built jar, or do we have to build it ourselves? As in releases on github latest tag is 1.0.0
@zefir6 thanks for pointing out. I've updated the readme.
The jars are published to GitHub as maven repo here: https://github.com/banzaicloud/spark-metrics/tree/master/maven-repo/releases/com/banzaicloud/spark-metrics_2.11/2.3-1.1.0