Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HUDI-210] hudi-support-prometheus-pushgateway #1931

Merged
merged 7 commits into from
Aug 9, 2020
Merged

[HUDI-210] hudi-support-prometheus-pushgateway #1931

merged 7 commits into from
Aug 9, 2020

Conversation

liujinhui1994
Copy link
Contributor

Tips

What is the purpose of the pull request

hudi support prometheus/pushgateway

Committer checklist

  • Has a corresponding JIRA in PR title & commit

  • Commit message is descriptive of the change

  • CI is green

  • Necessary doc changes done or have another open PR

  • For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.

@liujinhui1994
Copy link
Contributor Author

61960BDB-6E83-4086-BA4C-F0F0DBBC6722

@liujinhui1994
Copy link
Contributor Author

9C0310E1-BE19-4cbf-9476-5351C72908FC
BC04E798-8AFA-40a0-8FFF-43D6F89ED990

@liujinhui1994
Copy link
Contributor Author

@leesf Please review

@leesf leesf self-assigned this Aug 8, 2020
@leesf
Copy link
Contributor

leesf commented Aug 8, 2020

rerun tests

1 similar comment
@liujinhui1994
Copy link
Contributor Author

rerun tests

Copy link
Contributor

@leesf leesf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, merging.

@leesf leesf merged commit 6b349b7 into apache:master Aug 9, 2020
@sbernauer
Copy link
Contributor

Hi, thanks for this PR!
I added simpleclient-0.8.0.jar simpleclient_dropwizard-0.8.0.jar simpleclient_httpserver-0.8.0.jar and simpleclient_pushgateway-0.8.0.jar to the classpath. If i try to use the prometheus exporter i get the following error. Do you know why this happens?

Caused by: java.lang.NoSuchMethodError: io.prometheus.client.dropwizard.DropwizardExports.<init>(Lorg/apache/hudi/com/codahale/metrics/MetricRegistry;)V
        at org.apache.hudi.metrics.prometheus.PrometheusReporter.<init>(PrometheusReporter.java:49)
        at org.apache.hudi.metrics.MetricsReporterFactory.createReporter(MetricsReporterFactory.java:75)
        at org.apache.hudi.metrics.Metrics.<init>(Metrics.java:45)
        at org.apache.hudi.metrics.Metrics.init(Metrics.java:73)
        at org.apache.hudi.utilities.deltastreamer.HoodieDeltaStreamerMetrics.<init>(HoodieDeltaStreamerMetrics.java:44)
        at org.apache.hudi.utilities.deltastreamer.DeltaSync.syncOnce(DeltaSync.java:227)
        at org.apache.hudi.utilities.deltastreamer.HoodieDeltaStreamer$DeltaSyncService.lambda$startService$0(HoodieDeltaStreamer.java:579)
        at java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1604)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
        at java.lang.Thread.run(Thread.java:748)

@liujinhui1994
Copy link
Contributor Author

Are you using deltastreamer?
Use to recompile and try again

@sbernauer
Copy link
Contributor

sbernauer commented Aug 10, 2020

Yes, im using deltastreamer, freshly compiled from source of the master branch.
The new config:

    hoodie.metrics.on=true
    hoodie.metrics.reporter.type=PROMETHEUS

@liujinhui1994
Copy link
Contributor Author

Please use 'include'
and try to recompile

@sbernauer
Copy link
Contributor

sbernauer commented Aug 10, 2020

I'm sorry, i dont get it. I use mvn clean package -DskipTests -DskipITs -Dscala-2.12 for compilation. What do you mean by

Please use 'include'

Edit: Oh you mean adding the scope include in the pom for the 4 dependencies?

@liujinhui1994
Copy link
Contributor Author

yes

@sbernauer
Copy link
Contributor

My setup has no access to the internet, so i downloaded the libs from mvn central and included them in my docker images. Is the version 0.8.0 correct?

$ ls simpleclient*
simpleclient-0.8.0.jar  simpleclient_common-0.8.0.jar  simpleclient_dropwizard-0.8.0.jar  simpleclient_httpserver-0.8.0.jar  simpleclient_pushgateway-0.8.0.jar

@liujinhui1994
Copy link
Contributor Author

0.8.0 correct

@sbernauer
Copy link
Contributor

The constructor of DropwizardExports seems to be stable and I'm wondering what is going wrong here (It does not seem like a version mismatch to me). https://github.com/prometheus/client_java/blob/570f8bb70ae6f5a0a950dc563b68fb495d2f4d69/simpleclient_dropwizard/src/main/java/io/prometheus/client/dropwizard/DropwizardExports.java#L30
Mabye org/apache/hudi/com/codahale/metrics/MetricRegistry (see exception) instead of com/codahale/metrics/MetricRegistry is a problem?

@sbernauer
Copy link
Contributor

If i uncompress my simpleclient_dropwizard-0.8.0.jar and look inside the class all seems correct:

$ javap DropwizardExports.class 
Compiled from "DropwizardExports.java"
public class io.prometheus.client.dropwizard.DropwizardExports extends io.prometheus.client.Collector implements io.prometheus.client.Collector$Describable {
  public io.prometheus.client.dropwizard.DropwizardExports(com.codahale.metrics.MetricRegistry);
  public io.prometheus.client.dropwizard.DropwizardExports(com.codahale.metrics.MetricRegistry, io.prometheus.client.dropwizard.samplebuilder.SampleBuilder);
[...]
}

@liujinhui1994
Copy link
Contributor Author

If you use the include tag to include the required dependencies, it should be correct. Let me see why

@sbernauer
Copy link
Contributor

sbernauer commented Aug 10, 2020

I don't use the include option but instead guarantee that all drivers and executors have the libs and correctly use them via SPARK_DIST_CLASSPATH.

@liujinhui1994
Copy link
Contributor Author

It’s because I missed to configure less dependencies, it will be fixed soon
@sbernauer

@sbernauer
Copy link
Contributor

Thanks a lot @UZi5136225 for your fast response!

@liujinhui1994
Copy link
Contributor Author

#1942 @sbernauer You can try this PR

@sbernauer
Copy link
Contributor

Thanks @UZi5136225, this fixed the problem!
I dont understand why, because i added all the libs below in the correct version to the classpath. But it works :)

mvn dependency:tree | grep simpleclient
[INFO] +- io.prometheus:simpleclient:jar:0.8.0:compile
[INFO] +- io.prometheus:simpleclient_httpserver:jar:0.8.0:compile
[INFO] |  \- io.prometheus:simpleclient_common:jar:0.8.0:compile
[INFO] +- io.prometheus:simpleclient_dropwizard:jar:0.8.0:compile
[INFO] +- io.prometheus:simpleclient_pushgateway:jar:0.8.0:compile

@sbernauer
Copy link
Contributor

sbernauer commented Aug 10, 2020

@UZi5136225 i just noticed that i get a java.lang.ClassNotFoundException: io.prometheus.client.exporter.common.TextFormat when trying to access the interface. I think we should also add (i will give it a try)

<include>io.prometheus:simpleclient_common</include>

@liujinhui1994
Copy link
Contributor Author

Yes, this configuration needs to be added

@sbernauer
Copy link
Contributor

Hi @UZi5136225 it is currently not possible to set the metric with hoodie.metrics.prometheus.metric.prefix. Is this intended?

@leesf
Copy link
Contributor

leesf commented Aug 18, 2020

Hi @UZi5136225 it is currently not possible to set the metric with hoodie.metrics.prometheus.metric.prefix. Is this intended?

@UZi5136225 would you please take a look at this?

@liujinhui1994
Copy link
Contributor Author

Right Prometheus does not support, these are specially made for pushgateway
@sbernauer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants