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
metrics, spark: add support for telemetry mechanism in Spark integration #2528
Conversation
client/java/src/main/java/io/openlineage/client/metrics/MetricsResolver.java
Outdated
Show resolved
Hide resolved
integration/spark/app/src/main/java/io/openlineage/spark/agent/OpenLineageSparkListener.java
Show resolved
Hide resolved
client/java/src/main/java/io/openlineage/client/metrics/MetricsResolver.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left few single comments' feedback above. Overall I am really happy with the progress you do on the PR and the shape it becomes. Amazing job @mobuchowski 🥇
19ced62
to
4ccf656
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code looks really well and I appreciate the solution to extract timers for facet builders.
Did you experience any problems with testing? I couldn't find any new tests added. Would it make sense to see a debug facet filled with timers for each visitor and builder?
integration/spark/app/src/main/java/io/openlineage/spark/agent/OpenLineageSparkListener.java
Outdated
Show resolved
Hide resolved
d7d2637
to
43d6deb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor cosmetic comments + a question about thread.sleep
which I would love to avoid
integration/spark/app/src/test/java/io/openlineage/spark/agent/SparkGenericIntegrationTest.java
Outdated
Show resolved
Hide resolved
integration/spark/shared/src/main/java/io/openlineage/spark/agent/JobMetricsHolder.java
Outdated
Show resolved
Hide resolved
...rk/shared/src/main/java/io/openlineage/spark/agent/lifecycle/OpenLineageRunEventBuilder.java
Show resolved
Hide resolved
...ration/spark/app/src/test/java/io/openlineage/spark/agent/SparkContainerIntegrationTest.java
Outdated
Show resolved
Hide resolved
a411c2c
to
6d93dfe
Compare
Signed-off-by: Maciej Obuchowski <obuchowski.maciej@gmail.com>
6d93dfe
to
7067a88
Compare
OpenLineage#2528) Signed-off-by: Maciej Obuchowski <obuchowski.maciej@gmail.com> Signed-off-by: Fabio Manganiello <fabio@manganiello.tech>
This PR follows previous metrics PR #2496 and adds instrumentation to the Spark integration. This comprises of