Skip to content
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

chore: update telemetry events #6804

Merged
merged 2 commits into from
Jan 13, 2025
Merged

chore: update telemetry events #6804

merged 2 commits into from
Jan 13, 2025

Conversation

makeavish
Copy link
Member

@makeavish makeavish commented Jan 12, 2025

Summary

  1. Update alerts count to only count active alerts
  2. Update Query Range API to also include filters, groupby, aggregates info
  3. Add new events - Logs Explorer: Data present, Traces Explorer: Data present
  4. Fix false firing of traces sidebar event
  5. Fix logs count in last heart beat and metrics based panel
  6. Push total data and last heartbeat to profiles - Now it's possible to send identity events separately (was a blocker for segment deprecation on frontend)

Important

Update telemetry events to improve data accuracy, enhance Query Range API, and fix existing issues across multiple files.

  • Telemetry Enhancements:
    • Update alerts count to only include active alerts in db.go and response.go.
    • Add new telemetry events Logs Explorer: Data present and Traces Explorer: Data present in index.tsx and TimeSeriesView.tsx.
    • Fix false firing of traces sidebar event in Filter.tsx.
    • Push total data and last heartbeat to profiles in telemetry.go.
  • Query Range API:
    • Include filters, groupby, aggregates info in server.go.
    • Use CheckQueryInfo to determine query details and update telemetry data in server.go and http_handler.go.
  • Bug Fixes:
    • Fix logs count in last heartbeat and metrics-based panel in reader.go and model.go.
    • Correct metric-based panels count in model.go.

This description was created by Ellipsis for e157ab8. It will automatically update as commits are pushed.

Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

@github-actions github-actions bot added the chore label Jan 12, 2025
Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

1 similar comment
Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

@makeavish makeavish marked this pull request as ready for review January 13, 2025 09:15
@makeavish makeavish requested a review from YounixM as a code owner January 13, 2025 09:15
Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to e157ab8 in 2 minutes and 16 seconds

More details
  • Looked at 664 lines of code in 13 files
  • Skipped 0 files when reviewing.
  • Skipped posting 7 drafted comments based on config settings.
1. pkg/query-service/rules/db.go:620
  • Draft comment:
    The update to increment TotalActiveAlerts only when rule.PostableRule.Disabled is false correctly aligns with the intent to count only active alerts.
  • Reason this comment was not posted:
    Confidence changes required: 10%
    The change in the GetAlertsInfo function in pkg/query-service/rules/db.go correctly updates the TotalActiveAlerts count by checking if the alert is not disabled. This aligns with the PR description's intent to update alerts count to only count active alerts.
2. pkg/query-service/app/server.go:530
  • Draft comment:
    The addition of detailed query information (e.g., filterApplied, groupByApplied, aggregateOperator, etc.) to the telemetry data aligns with the intent to provide more comprehensive query insights.
  • Reason this comment was not posted:
    Confidence changes required: 10%
    The change in extractQueryRangeData and extractQueryRangeV3Data functions in pkg/query-service/app/server.go ensures that the telemetry data includes additional query information such as filters, groupby, and aggregates. This aligns with the PR description's intent to update Query Range API to include more detailed information.
3. pkg/query-service/telemetry/telemetry.go:170
  • Draft comment:
    The CheckQueryInfo function now correctly extracts detailed query information, which is used for telemetry purposes. This aligns with the intent to provide more comprehensive query insights.
  • Reason this comment was not posted:
    Confidence changes required: 10%
    The change in CheckQueryInfo function in pkg/query-service/telemetry/telemetry.go correctly extracts and returns detailed query information, which is then used in telemetry events. This aligns with the PR description's intent to update Query Range API to include more detailed information.
4. frontend/src/container/LogsExplorerList/index.tsx:72
  • Draft comment:
    The addition of a telemetry event for 'Logs Explorer: Data present' when data is available aligns with the intent to track data presence in explorers.
  • Reason this comment was not posted:
    Confidence changes required: 10%
    The change in frontend/src/container/LogsExplorerList/index.tsx adds a new telemetry event when data is present in the logs explorer. This aligns with the PR description's intent to add new events for data presence in explorers.
5. frontend/src/container/TracesExplorer/ListView/index.tsx:146
  • Draft comment:
    The addition of a telemetry event for 'Traces Explorer: Data present' when data is available aligns with the intent to track data presence in explorers.
  • Reason this comment was not posted:
    Confidence changes required: 10%
    The change in frontend/src/container/TracesExplorer/ListView/index.tsx adds a new telemetry event when data is present in the traces explorer. This aligns with the PR description's intent to add new events for data presence in explorers.
6. frontend/src/container/TimeSeriesView/TimeSeriesView.tsx:121
  • Draft comment:
    The addition of telemetry events for 'Traces Explorer: Data present' and 'Logs Explorer: Data present' when data is available aligns with the intent to track data presence in explorers.
  • Reason this comment was not posted:
    Confidence changes required: 10%
    The change in frontend/src/container/TimeSeriesView/TimeSeriesView.tsx adds a new telemetry event when data is present in the time series view. This aligns with the PR description's intent to add new events for data presence in explorers.
7. frontend/src/container/LogsExplorerList/index.tsx:1
  • Draft comment:
    Avoid using the component/index.tsx file structure approach, as it makes it difficult to debug and find components using global search tools like VS Code. This is also applicable to other index.tsx files in the PR.
  • Reason this comment was not posted:
    Comment was on unchanged code.

Workflow ID: wflow_JMewz0jrwocLsIcM


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@SagarRajput-7 SagarRajput-7 left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Member

@prashant-shahi prashant-shahi left a comment

Choose a reason for hiding this comment

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

:shipit: LGTM

@makeavish makeavish merged commit a60371f into main Jan 13, 2025
17 of 20 checks passed
@makeavish makeavish deleted the chore/event-changes branch January 13, 2025 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants