Skip to content

Add telemetry related to datafusion changes - #1070

Closed
rishabh6788 wants to merge 0 commit into
opensearch-project:mainfrom
rishabh6788:main
Closed

Add telemetry related to datafusion changes#1070
rishabh6788 wants to merge 0 commit into
opensearch-project:mainfrom
rishabh6788:main

Conversation

@rishabh6788

@rishabh6788 rishabh6788 commented May 18, 2026

Copy link
Copy Markdown
Collaborator

Description

As part of datafusion related changes going on OpenSearch core, the team has requested to add new telemetry device which would fetch and ingest metrics related for backend analytics plugin.

Usage: pass --telemetry datafusion-stats in the command.

Below is how the stats will be queried:

curl -s 'http://localhost:9200/_plugins/analytics_backend_datafusion/stats?pretty'
{
  "io_runtime" : {
    "workers_count" : 16,
    "total_polls_count" : 0,
    "total_busy_duration_ms" : 0,
    "total_overflow_count" : 0,
    "global_queue_depth" : 0,
    "blocking_queue_depth" : 0,
    "num_alive_tasks" : 0,
    "spawned_tasks_count" : 0,
    "total_local_queue_depth" : 0
  },
  "cpu_runtime" : {
    "workers_count" : 8,
    "total_polls_count" : 0,
    "total_busy_duration_ms" : 0,
    "total_overflow_count" : 0,
    "global_queue_depth" : 0,
    "blocking_queue_depth" : 0,
    "num_alive_tasks" : 0,
    "spawned_tasks_count" : 0,
    "total_local_queue_depth" : 0
  },
  "coordinator_reduce" : {
    "total_poll_duration_ms" : 0,
    "total_scheduled_duration_ms" : 0,
    "total_idle_duration_ms" : 0
  },
  "query_execution" : {
    "total_poll_duration_ms" : 7,
    "total_scheduled_duration_ms" : 0,
    "total_idle_duration_ms" : 0
  },
  "stream_next" : {
    "total_poll_duration_ms" : 0,
    "total_scheduled_duration_ms" : 0,
    "total_idle_duration_ms" : 0
  },
  "plan_setup" : {
    "total_poll_duration_ms" : 34,
    "total_scheduled_duration_ms" : 0,
    "total_idle_duration_ms" : 3
  }
}

Issues Resolved

[List any issues this PR will resolve]

Testing

  • New functionality includes testing

[Describe how this change was tested]


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@github-actions

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ Recommended focus areas for review

Possible Issue

The flattening logic in record() only processes dict values but does not handle non-dict top-level values. If the API response includes a top-level key with a non-dict value (e.g., a string or number), that key-value pair is silently ignored and not stored in the metrics. This could lead to incomplete telemetry data if the API format changes or includes additional fields.

for section_name, section_values in stats.items():
    if isinstance(section_values, dict):
        for key, value in section_values.items():
            doc[f"{section_name}_{key}"] = value

@github-actions

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Preserve top-level scalar values from stats

The code assumes all non-dict values in stats should be ignored, but doesn't handle
top-level scalar values. If the API returns unexpected structure with top-level
scalars, they will be silently dropped, potentially losing important data.

osbenchmark/telemetry.py [2020-2024]

 doc = {"name": "datafusion-stats"}
 for section_name, section_values in stats.items():
     if isinstance(section_values, dict):
         for key, value in section_values.items():
             doc[f"{section_name}_{key}"] = value
+    else:
+        doc[section_name] = section_values
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies that top-level scalar values would be silently dropped. However, based on the test data in DataFusionStatsRecorderTests.stats_response, the API only returns nested dictionaries, making this a defensive improvement rather than fixing an actual bug.

Low

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.

1 participant