Skip to content

feat: expose comet metrics through Sparks external monitoring system#3708

Merged
parthchandra merged 6 commits into
apache:mainfrom
coderfender:project_comet_metrics
Mar 30, 2026
Merged

feat: expose comet metrics through Sparks external monitoring system#3708
parthchandra merged 6 commits into
apache:mainfrom
coderfender:project_comet_metrics

Conversation

@coderfender
Copy link
Copy Markdown
Contributor

@coderfender coderfender commented Mar 15, 2026 •
edited
Loading

Which issue does this PR close?

Closes #3712

Rationale for this change

  1. We currently collect metrics and show while logging in comet. This PR is to essentially go a step further and wire the metrics out to be visualized in comet through tools like Grafana . As more users start using comet, these metrics would be helpful in understanding acceleration from comet standpoint. These are first Comet metrics exposed to Spark's external monitoring system and more would follow

The following metrics are exposed at the moment :

  1. comet.operators.native
  2. comet.operators.spark
  3. comet.queries.planned
  4. comet.transitions
  5. comet.acceleration.ratio (acceleration : native / (native + spark)

What changes are included in this PR?

How are these changes tested?

@coderfender coderfender force-pushed the project_comet_metrics branch from aa97dec to f096d8f Compare March 15, 2026 22:02
@coderfender
Copy link
Copy Markdown
Contributor Author

coderfender commented Mar 15, 2026

cc : @andygrove

@coderfender coderfender changed the title feat: expose comet metrics for visualization feat: expose comet metrics through Sparks external monitoring system Mar 16, 2026
wForget
wForget reviewed Mar 16, 2026
View reviewed changes
Comment thread spark/src/main/scala/org/apache/comet/rules/CometExecRule.scala Outdated
@coderfender coderfender force-pushed the project_comet_metrics branch from f096d8f to c0eb149 Compare March 16, 2026 03:03
@coderfender
Copy link
Copy Markdown
Contributor Author

coderfender commented Mar 16, 2026

@wForget , Please take a look whenever you get a chance

@coderfender coderfender force-pushed the project_comet_metrics branch from c0eb149 to 2eb506f Compare March 16, 2026 15:52
wForget
wForget reviewed Mar 17, 2026
View reviewed changes
Comment thread spark/src/main/scala/org/apache/comet/rules/CometExecRule.scala Outdated
wForget
wForget reviewed Mar 17, 2026
View reviewed changes
Comment thread spark/src/test/scala/org/apache/spark/CometPluginsSuite.scala
@coderfender coderfender force-pushed the project_comet_metrics branch from 8c60dc8 to eaa1dc4 Compare March 17, 2026 08:10
@coderfender
Copy link
Copy Markdown
Contributor Author

coderfender commented Mar 17, 2026

@wForget , I addressed code per your review comments. Please take a look whenever you get a chance. Thank you for the kind guidance

wForget
wForget reviewed Mar 17, 2026
View reviewed changes
Comment thread spark/src/test/scala/org/apache/spark/CometPluginsSuite.scala Outdated
wForget
wForget reviewed Mar 17, 2026
View reviewed changes
Comment thread spark/src/main/scala/org/apache/spark/Plugins.scala Outdated
parthchandra
parthchandra reviewed Mar 17, 2026
View reviewed changes
Comment thread spark/src/main/scala/org/apache/spark/Plugins.scala Outdated
Comment thread spark/src/main/scala/org/apache/spark/CometSource.scala
@coderfender
Copy link
Copy Markdown
Contributor Author

coderfender commented Mar 18, 2026

Thwnk you for the review . Let me take a look and update the logic

@coderfender coderfender force-pushed the project_comet_metrics branch from c6da902 to 2b6dcc1 Compare March 18, 2026 22:06
wForget
wForget reviewed Mar 20, 2026
View reviewed changes
Comment thread spark/src/main/scala/org/apache/spark/Plugins.scala
wForget
wForget reviewed Mar 20, 2026
View reviewed changes
Comment thread spark/src/main/scala/org/apache/comet/CometMetricsListener.scala
@wForget
Copy link
Copy Markdown
Member

wForget commented Mar 20, 2026

Thanks @coderfender , overall looks good to me; just need to add a configuration to control this behavior, and keep it disabled by default.

@coderfender
Copy link
Copy Markdown
Contributor Author

coderfender commented Mar 20, 2026 •
edited
Loading

@wForget , thank you for the approval. My preference here is to enable metrics reporting by default unless you think that we would be compromising performance in collecting the metrics ?

@wForget
Copy link
Copy Markdown
Member

wForget commented Mar 23, 2026

My preference here is to enable metrics reporting by default unless you think that we would be compromising performance in collecting the metrics ?

  1. Spark metrics are not always collected; they typically require a metrics system sink like Prometheus.
  2. Spark plan traversal is not lightweight enough, they may have many nodes.

@coderfender coderfender force-pushed the project_comet_metrics branch from 523db1d to 6b22cdc Compare March 27, 2026 04:01
@coderfender
Copy link
Copy Markdown
Contributor Author

coderfender commented Mar 27, 2026

Rebased with main @wForget please take a look whenever you get a chance

wForget
wForget approved these changes Mar 27, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@wForget wForget left a comment

Choose a reason for hiding this comment

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

Thanks @coderfender , LGTM

@coderfender
Copy link
Copy Markdown
Contributor Author

coderfender commented Mar 27, 2026

Thank you @wForget

parthchandra
parthchandra approved these changes Mar 27, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@parthchandra parthchandra left a comment

Choose a reason for hiding this comment

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

lgtm, some minor comments you might consider.

Comment thread common/src/main/scala/org/apache/comet/CometConf.scala Outdated
Comment thread spark/src/main/scala/org/apache/comet/CometMetricsListener.scala Outdated
@coderfender
Copy link
Copy Markdown
Contributor Author

coderfender commented Mar 28, 2026

@parthchandra , I made changes per recommendation . Please take a look whenever you get a chance . Thank you

@parthchandra parthchandra merged commit bef7759 into apache:main Mar 30, 2026
158 checks passed
@parthchandra
Copy link
Copy Markdown
Contributor

parthchandra commented Mar 30, 2026

Merged. Thanks @coderfender

vaibhawvipul pushed a commit to vaibhawvipul/datafusion-comet that referenced this pull request Apr 4, 2026
@coderfender @vaibhawvipul
feat: expose comet metrics through Sparks external monitoring system (a…
6fa63b1 
…pache#3708)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@parthchandra parthchandra parthchandra approved these changes

@wForget wForget wForget approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Export comet metrics externally through spark's monitoring system

3 participants

@coderfender @wForget @parthchandra