chore: update heartbeat, language event and add serviceName event #4571
Conversation
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
WalkthroughThe recent updates enhance the query service's capability to handle telemetry data more flexibly. By introducing new methods and modifying existing ones, the service can now query logs, samples, spans, and tags over specific intervals, improving data retrieval efficiency. Additionally, the telemetry system has been refined to better capture and report on service-related metrics, ensuring a more comprehensive understanding of system performance and user interactions. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
2 similar comments
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 5
Configuration used: CodeRabbit UI
Files selected for processing (3)
- pkg/query-service/app/clickhouseReader/reader.go (6 hunks)
- pkg/query-service/interfaces/interface.go (2 hunks)
- pkg/query-service/telemetry/telemetry.go (7 hunks)
Additional comments: 10
pkg/query-service/interfaces/interface.go (5)
- 5-5: The addition of the
timepackage import is appropriate given the introduction of methods that require atime.Durationparameter. This change is necessary for the functionality being introduced and follows best practices for handling time-based operations in Go.- 78-78: The method
GetTotalLogshas been correctly renamed and now returns auint64and anerror, which is a standard approach for methods that perform operations which might fail. This change aligns with Go's idiomatic error handling.- 79-79: The addition of the
GetTotalSamplesmethod is consistent with the objective of enhancing telemetry data collection capabilities. Returning auint64and anerroris appropriate for the expected functionality.- 80-80: The modification of
GetSpansInLastHeartBeatIntervalto include anintervalparameter of typetime.Durationis logical and necessary for the intended functionality of querying spans within a specific time interval. This change improves the method's flexibility and utility.- 82-84: The updates to
GetSamplesInfoInLastHeartBeatInterval,GetLogsInfoInLastHeartBeatInterval, andGetTagsInfoInLastHeartBeatIntervalmethods to include anintervalparameter of typetime.Durationare consistent with the objective of enhancing query capabilities based on specific time intervals. These changes are correctly implemented and follow best practices for handling time-based operations in Go.pkg/query-service/telemetry/telemetry.go (4)
- 38-38: The addition of the
TELEMETRY_EVENT_SERVICEconstant is correctly implemented and follows Go's naming conventions for constants. This change is necessary for the new functionality being introduced.- 55-55: Updating the
SAAS_EVENTS_LISTmap to include theTELEMETRY_EVENT_SERVICEconstant is appropriate and necessary for the new telemetry event handling. This change ensures that the new event type is recognized and processed correctly.- 235-237: The loop that sends the
TELEMETRY_EVENT_SERVICEevent for each service found intagsInfo.Servicesis correctly implemented. This approach allows for the emission of service-specific telemetry events, enhancing the granularity of the telemetry data collected.- 288-288: The conditional check to send the
TELEMETRY_EVENT_DASHBOARDS_ALERTSevent only if there are dashboards or alerts is a good practice. It avoids unnecessary event emissions when there is no relevant data to report, which can help in optimizing the telemetry data collection process.pkg/query-service/app/clickhouseReader/reader.go (1)
- 3365-3368: The method
GetTotalSamplesretrieves the total count of samples excluding those with a metric name starting with 'signoz_'. This is a straightforward query, but it's important to ensure that the metric name filter ('signoz_%') correctly captures all intended cases and doesn't inadvertently exclude relevant data. Additionally, consider the performance impact of this query on large datasets and whether any optimizations or indexes could improve its efficiency.
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
There was a problem hiding this comment.
Had a code walk-through with @makeavish - approving on request
Summary
Summary by CodeRabbit