chore: added telemetry to the plugin flow#39861
Conversation
WalkthroughThis pull request introduces new observability features across several plugins and updates the tracing constants. It adds five span constants in the tracing interface and integrates Micrometer’s ObservationRegistry into the GoogleSheets, Mongo, MySQL, Postgres, and RestApi plugins. The changes update constructor signatures and method chains to incorporate observation calls and modify test instantiations to provide ObservationRegistry.NOOP. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant E as PluginExecutor
participant R as ObservationRegistry
participant M as Micrometer
C->>E: executeParameterized(parameters)
E->>R: Initialize observation
E->>E: Execute common logic
E->>M: Chain observation (.name, .tap)
M-->>E: Return observation metrics
E-->>C: Return ActionExecutionResult
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
bot
added
the
skip-changelog
app/server/appsmith-plugins/restApiPlugin/src/main/java/com/external/plugins/RestApiPlugin.java
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/plugins/GoogleSheetsPlugin.java (1)
354-417: Missing telemetry in triggerWithFlags method.The triggerWithFlags method would benefit from similar observation instrumentation as applied to executeParameterizedWithFlags.
Consider adding observation to the exchange call in triggerWithFlags:
.exchange() +.name(ACTUAL_API_CALL) +.tap(Micrometer.observation(observationRegistry)) .flatMap(clientResponse -> clientResponse.toEntity(byte[].class))
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (12)
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/ActionSpanCE.java(1 hunks)app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/RestAPIActivateUtils.java(5 hunks)app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/plugins/GoogleSheetsPlugin.java(6 hunks)app/server/appsmith-plugins/graphqlPlugin/src/main/java/com/external/plugins/GraphQLPlugin.java(2 hunks)app/server/appsmith-plugins/mongoPlugin/src/main/java/com/external/plugins/MongoPlugin.java(6 hunks)app/server/appsmith-plugins/mssqlPlugin/src/main/java/com/external/plugins/MssqlPlugin.java(4 hunks)app/server/appsmith-plugins/mssqlPlugin/src/test/java/com/external/plugins/MssqlTestDBContainerManager.java(2 hunks)app/server/appsmith-plugins/mysqlPlugin/src/main/java/com/external/plugins/MySqlPlugin.java(7 hunks)app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/PostgresPlugin.java(5 hunks)app/server/appsmith-plugins/postgresPlugin/src/test/java/com/external/plugins/PostgresDatasourceValidationTest.java(2 hunks)app/server/appsmith-plugins/postgresPlugin/src/test/java/com/external/plugins/PostgresPluginTest.java(2 hunks)app/server/appsmith-plugins/restApiPlugin/src/main/java/com/external/plugins/RestApiPlugin.java(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app/server/appsmith-plugins/restApiPlugin/src/main/java/com/external/plugins/RestApiPlugin.java
- app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/PostgresPlugin.java
🧰 Additional context used
🧬 Code Definitions (5)
app/server/appsmith-plugins/mysqlPlugin/src/main/java/com/external/plugins/MySqlPlugin.java (1)
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/ActionSpanCE.java (1)
ActionSpanCE(11-72)
app/server/appsmith-plugins/mssqlPlugin/src/main/java/com/external/plugins/MssqlPlugin.java (1)
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/ActionSpanCE.java (1)
ActionSpanCE(11-72)
app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/plugins/GoogleSheetsPlugin.java (1)
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/ActionSpanCE.java (1)
ActionSpanCE(11-72)
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/RestAPIActivateUtils.java (1)
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/ActionSpanCE.java (1)
ActionSpanCE(11-72)
app/server/appsmith-plugins/mongoPlugin/src/main/java/com/external/plugins/MongoPlugin.java (1)
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/ActionSpanCE.java (1)
ActionSpanCE(11-72)
⏰ Context from checks skipped due to timeout of 90000ms (60)
- GitHub Check: perform-test / ci-test / ci-test (59)
- GitHub Check: perform-test / ci-test / ci-test (58)
- GitHub Check: perform-test / ci-test / ci-test (57)
- GitHub Check: perform-test / ci-test / ci-test (56)
- GitHub Check: perform-test / ci-test / ci-test (55)
- GitHub Check: perform-test / ci-test / ci-test (54)
- GitHub Check: perform-test / ci-test / ci-test (53)
- GitHub Check: perform-test / ci-test / ci-test (52)
- GitHub Check: perform-test / ci-test / ci-test (51)
- GitHub Check: perform-test / ci-test / ci-test (50)
- GitHub Check: perform-test / ci-test / ci-test (49)
- GitHub Check: perform-test / ci-test / ci-test (48)
- GitHub Check: perform-test / ci-test / ci-test (47)
- GitHub Check: perform-test / ci-test / ci-test (46)
- GitHub Check: perform-test / ci-test / ci-test (45)
- GitHub Check: perform-test / ci-test / ci-test (44)
- GitHub Check: perform-test / ci-test / ci-test (43)
- GitHub Check: perform-test / ci-test / ci-test (41)
- GitHub Check: perform-test / ci-test / ci-test (40)
- GitHub Check: perform-test / ci-test / ci-test (39)
- GitHub Check: perform-test / ci-test / ci-test (38)
- GitHub Check: perform-test / ci-test / ci-test (37)
- GitHub Check: perform-test / ci-test / ci-test (36)
- GitHub Check: perform-test / ci-test / ci-test (35)
- GitHub Check: perform-test / ci-test / ci-test (34)
- GitHub Check: perform-test / ci-test / ci-test (33)
- GitHub Check: perform-test / ci-test / ci-test (32)
- GitHub Check: perform-test / ci-test / ci-test (31)
- GitHub Check: perform-test / ci-test / ci-test (30)
- GitHub Check: perform-test / ci-test / ci-test (29)
- GitHub Check: perform-test / ci-test / ci-test (28)
- GitHub Check: perform-test / ci-test / ci-test (27)
- GitHub Check: perform-test / ci-test / ci-test (26)
- GitHub Check: perform-test / ci-test / ci-test (25)
- GitHub Check: perform-test / ci-test / ci-test (24)
- GitHub Check: perform-test / ci-test / ci-test (23)
- GitHub Check: perform-test / ci-test / ci-test (22)
- GitHub Check: perform-test / ci-test / ci-test (21)
- GitHub Check: perform-test / ci-test / ci-test (20)
- GitHub Check: perform-test / ci-test / ci-test (19)
- GitHub Check: perform-test / ci-test / ci-test (18)
- GitHub Check: perform-test / ci-test / ci-test (17)
- GitHub Check: perform-test / ci-test / ci-test (16)
- GitHub Check: perform-test / ci-test / ci-test (15)
- GitHub Check: perform-test / ci-test / ci-test (14)
- GitHub Check: perform-test / ci-test / ci-test (13)
- GitHub Check: perform-test / ci-test / ci-test (12)
- GitHub Check: perform-test / ci-test / ci-test (11)
- GitHub Check: perform-test / ci-test / ci-test (10)
- GitHub Check: perform-test / ci-test / ci-test (9)
- GitHub Check: perform-test / ci-test / ci-test (8)
- GitHub Check: perform-test / ci-test / ci-test (7)
- GitHub Check: perform-test / ci-test / ci-test (6)
- GitHub Check: perform-test / ci-test / ci-test (5)
- GitHub Check: perform-test / ci-test / ci-test (4)
- GitHub Check: perform-test / ci-test / ci-test (3)
- GitHub Check: perform-test / ci-test / ci-test (2)
- GitHub Check: perform-test / ci-test / ci-test (1)
- GitHub Check: perform-test / ci-test / ci-test (0)
- GitHub Check: server-unit-tests / server-unit-tests
🔇 Additional comments (50)
app/server/appsmith-plugins/postgresPlugin/src/test/java/com/external/plugins/PostgresDatasourceValidationTest.java (4)
14-14: Import added for ObservationRegistry.The import statement is correctly added to support the updated constructor parameters.
33-34: Constructor updated to include ObservationRegistry parameter.The
PostgresPluginExecutorinstantiation now includesObservationRegistry.NOOPas required by the modified constructor signature. This is part of the telemetry implementation being added to the plugin flow.
14-14: Added import for ObservationRegistry.The added import aligns with the updated constructor parameter introduced below.
33-34: Updated constructor call to include ObservationRegistry.The PostgresPluginExecutor now requires an observation registry parameter, which is correctly provided with ObservationRegistry.NOOP for testing purposes.
app/server/appsmith-plugins/postgresPlugin/src/test/java/com/external/plugins/PostgresPluginTest.java (4)
32-32: Import added for ObservationRegistry.The import statement is correctly added to support the updated constructor parameters.
92-93: Constructor updated to include ObservationRegistry parameter.The
PostgresPluginExecutorinstantiation now includesObservationRegistry.NOOPas required by the modified constructor signature. This is consistent with the changes made toPostgresDatasourceValidationTest.
32-32: Added import for ObservationRegistry.This import supports the updated constructor parameter.
92-93: Updated constructor call with ObservationRegistry parameter.The executor instantiation now includes ObservationRegistry.NOOP as the third parameter, consistent with the constructor signature change.
app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/plugins/GoogleSheetsPlugin.java (9)
31-31: New imports for observation-related dependencies.Required imports added for the Micrometer observation registry implementation.
Also applies to: 40-40
51-52: Static imports for span constants.These constants are correctly imported and will be used to name the observation spans for better traceability.
82-86: Observation registry field and constructor added.The field and constructor properly initialize the observation registry, which will be used for telemetry. Good practice to use constructor injection for this dependency.
163-165: Added telemetry instrumentation to executeParameterizedWithFlags method.The method correctly implements telemetry by naming the observation with
PLUGIN_EXECUTE_COMMONand tapping into the Micrometer observation API.
225-226: Added telemetry instrumentation to the actual API call.The HTTP exchange is properly instrumented with the
ACTUAL_API_CALLname for observability. This will help track API call performance.
31-31: Added necessary imports for telemetry implementation.The imports support the Micrometer observation features and define constants for span names.
Also applies to: 40-40, 51-52
82-86: Added ObservationRegistry field and constructor.The executor now properly stores the observation registry instance for use in instrumentation.
163-165: Added observation to executeCommon method call.Properly instruments the executeCommon method with a named observation span.
225-226: Added observation to API call exchange.Instruments the actual API call with an observation span for better telemetry.
app/server/appsmith-plugins/mssqlPlugin/src/test/java/com/external/plugins/MssqlTestDBContainerManager.java (4)
10-10: Import added for ObservationRegistry.Import statement correctly added to support the modified constructor parameters.
32-32: Constructor updated to include ObservationRegistry parameter.The
MssqlPluginExecutorinstantiation now includesObservationRegistry.NOOPas required by the updated constructor signature, consistent with the pattern applied to other plugin executors.
10-10: Added import for ObservationRegistry.Supports the constructor parameter addition below.
31-32: Updated MssqlPluginExecutor instantiation with ObservationRegistry.Correctly provides ObservationRegistry.NOOP for testing purposes, matching the constructor signature change.
app/server/appsmith-plugins/mssqlPlugin/src/main/java/com/external/plugins/MssqlPlugin.java (5)
5-5: Adding import for ActionSpanCE to support telemetry.This import is correctly added to access the
PLUGIN_EXECUTE_COMMONconstant used in the updated code.
37-38: Adding Micrometer imports for observability.These imports are necessary for the new telemetry functionality being added to the plugin. The ObservationRegistry and Micrometer imports are essential for the observability chain.
Also applies to: 45-45
123-124: Adding ObservationRegistry field.Adding this private final field to store the registry instance is the correct approach for making it available throughout the class.
125-128: Constructor updated to accept ObservationRegistry.The constructor has been updated to accept and store the ObservationRegistry, maintaining good dependency injection practices.
190-193: Adding telemetry to query execution.This enhancement adds observability to the query execution flow by naming the operation and tapping into the observation registry. This is a good practice for monitoring plugin performance.
app/server/appsmith-plugins/mysqlPlugin/src/main/java/com/external/plugins/MySqlPlugin.java (7)
38-38: Adding ObservationRegistry import.This import is necessary for the new telemetry functionality.
57-57: Adding Micrometer import for observability.This import provides the
Micrometer.observation()method used to enhance telemetry.
77-78: Importing ActionSpanCE constants.These static imports provide the span names used for telemetry tracking in the plugin execution.
171-176: Adding ObservationRegistry field and constructor parameter.Properly adding the ObservationRegistry as a dependency through constructor injection.
242-244: Adding telemetry to non-prepared statement execution path.Enhancing observability for the non-prepared statement execution path by naming the operation and adding an observation tap.
261-262: Adding telemetry to prepared statement execution path.Ensuring both execution paths (prepared and non-prepared statements) have consistent telemetry implementation.
367-368: Adding telemetry to query creation and execution.This adds detailed telemetry for the specific query creation and execution phase, providing more granular performance tracking.
app/server/appsmith-plugins/graphqlPlugin/src/main/java/com/external/plugins/GraphQLPlugin.java (2)
25-25: Adding ObservationRegistry import.This import is needed for the new parameter passed to the
triggerApiCallmethod.
288-289: Adding ObservationRegistry.NOOP to triggerApiCall method.This change passes a no-operation observation registry to the triggerApiCall method, maintaining consistency with the updated method signature without enabling active telemetry in this specific context.
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/ActionSpanCE.java (3)
20-21: Adding constants for plugin execution spans.These constants define span names for common plugin execution operations and MongoDB output tracking.
23-24: Adding constant for query execution span.This constant defines the span name for tracking the query creation and execution phase across database plugins.
26-27: Adding constants for API call spans.These constants define span names for tracking actual API calls and API call triggers, enhancing observability for API-based plugins.
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/RestAPIActivateUtils.java (5)
20-21: Good addition of ObservationRegistry importThe addition of Micrometer's ObservationRegistry is appropriate for implementing telemetry in the REST API component.
33-34: Properly imported Micrometer observabilityThe Micrometer reactive observability import is essential for integrating with the observation registry.
50-51: Good practice using constant for span namingUsing a predefined constant from ActionSpanCE for naming the span ensures consistency across the codebase.
79-80: Method signature updated correctlyThe method signature has been properly updated to include the ObservationRegistry parameter, maintaining backward compatibility with the method logic.
81-84: Well-implemented observability patternThe implementation follows the standard reactive observability pattern with proper naming and tap operations. This will provide valuable telemetry data for API calls.
app/server/appsmith-plugins/mongoPlugin/src/main/java/com/external/plugins/MongoPlugin.java (7)
54-55: Good addition of ObservationRegistry importThe ObservationRegistry import enables telemetry collection for MongoDB operations.
74-75: Properly imported Micrometer observabilityThe reactive Micrometer observability import aligns with the project's telemetry approach.
102-104: Good practice using constants for span namingImporting constants for span names from ActionSpanCE ensures naming consistency across plugins.
230-235: Well-implemented constructor injectionThe addition of the ObservationRegistry field and constructor properly implements dependency injection, allowing for consistent telemetry collection.
299-302: Properly instrumented executeCommon methodThe instrumentation of the executeCommon method call with the PLUGIN_EXECUTE_COMMON span name will provide visibility into plugin execution time and frequency.
348-351: Effectively instrumented MongoDB operationAdding observability to the mongoOutputMono will provide valuable metrics about MongoDB command execution performance, which is especially useful for identifying slow queries.
1-1178:Details
✅ Verification successful
Verify test coverage for new telemetry features
The telemetry additions are well-implemented, but ensure that tests have been updated to verify telemetry is correctly emitted. ObservationRegistry.NOOP should be used for tests to avoid unintended side effects.
🏁 Script executed:
#!/bin/bash # Check if tests are passing and properly using ObservationRegistry.NOOP # Search for test files that may need updating with the new ObservationRegistry parameter echo "Looking for test files that may need updates for ObservationRegistry..." rg -l "MongoPluginExecutor|RestAPIActivateUtils" --glob="*Test.java" --glob="*Tests.java" # Check if tests are using ObservationRegistry.NOOP echo "Checking if tests are using ObservationRegistry.NOOP..." rg "ObservationRegistry.NOOP" --glob="*Test.java" --glob="*Tests.java"Length of output: 4800
Telemetry Test Coverage Verified for Mongo Plugin
The tests have been updated to correctly utilizeObservationRegistry.NOOP. In particular, files likeMongoPluginStaleConnTest.java,MongoPluginDatasourceTest.java,MongoPluginRegexTest.java,MongoPluginQueriesTest.java, andMongoPluginFormsTest.javaproperly instantiate the plugin executor withObservationRegistry.NOOP, ensuring that telemetry data is emitted as expected without side effects.
rajatagrawal
added
ok-to-test
## Description added telemetry to the plugin flow Fixes #`Issue Number` _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /ok-to-test tags="@tag.All" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!CAUTION] > 🔴 🔴 🔴 Some tests have failed. > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/14052770190> > Commit: e7102de > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=14052770190&attempt=2&selectiontype=test&testsstatus=failed&specsstatus=fail" target="_blank">Cypress dashboard</a>. > Tags: @tag.All > Spec: > The following are new failures, please fix them before merging the PR: <ol> > <li>cypress/e2e/Regression/ClientSide/Widgets/Custom/CustomWidgetEditorPropertyPane_spec.ts</ol> > <a href="https://internal.appsmith.com/app/cypress-dashboard/identified-flaky-tests-65890b3c81d7400d08fa9ee3?branch=master" target="_blank">List of identified flaky tests</a>. > <hr>Tue, 25 Mar 2025 08:51:35 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced new tracing constants for improved action tracking. - Enhanced observability across multiple plugins (for databases and APIs) by integrating monitoring capabilities into their execution flows. - **Tests** - Updated test setups to initialize plugins with observation support, ensuring consistency with the enhanced monitoring features. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 participants
Description
added telemetry to the plugin flow
Fixes #
Issue Numberor
Fixes
Issue URLWarning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.All"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/14052770190
Commit: e7102de
Cypress dashboard.
Tags:
@tag.AllSpec:
Tue, 25 Mar 2025 09:04:21 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Tests