Skip to content

fix: OSS telemetry for the number of services #7908

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

Merged
merged 2 commits into from
May 13, 2025

Conversation

Nageshbansal
Copy link
Member

@Nageshbansal Nageshbansal commented May 13, 2025

πŸ“„ Summary

Fixes the OSS telemetry for sending the Number of services events.


βœ… Changes

  • Feature: Brief description
  • Bug fix: Brief description

πŸ‘€ Notes for Reviewers


Important

Fixes error handling in GetWaterfallSpansForTraceWithMetadata in http_handler.go by correcting the UUID conversion error check.

  • Bug Fix:
    • Corrects error handling in GetWaterfallSpansForTraceWithMetadata in http_handler.go by changing if err != nil to if err == nil to properly handle UUID conversion errors.

This description was created by Ellipsis for a145148. You can customize this summary. It will automatically update as commits are pushed.

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.

Important

Looks good to me! πŸ‘

Reviewed everything up to a145148 in 1 minute and 13 seconds. Click for details.
  • Reviewed 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with πŸ‘ or πŸ‘Ž to teach Ellipsis.
1. pkg/query-service/app/http_handler.go:1710
  • Draft comment:
    BUG: The condition for sending the telemetry event is inverted. It currently checks if errv2 != nil before using claims.Email; it should be 'if errv2 == nil' to ensure claims was successfully retrieved.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. pkg/query-service/app/http_handler.go:115
  • Draft comment:
    Typo in the comment for SetupCompleted: instead of "is registers", please change it to "is registered".
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. pkg/query-service/app/http_handler.go:5055
  • Draft comment:
    Fix typographical error in the comment: change "experssion" to "expressions" in the comment explaining that functions are applied because expressions are executed directly in ClickHouse.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_8fazsoc1KdLFfcqq

You can customize Ellipsis by changing your verbosity settings, reacting with πŸ‘ or πŸ‘Ž, replying to comments, or adding code review rules.

@Nageshbansal Nageshbansal changed the title chore: Fix OSS telemetry for the number of services fix: OSS telemetry for the number of services May 13, 2025
@grandwizard28 grandwizard28 added the safe-to-test Run CI tests for dependabot and external contributors label May 13, 2025
@primus-bot primus-bot bot removed the safe-to-test Run CI tests for dependabot and external contributors label May 13, 2025
@grandwizard28 grandwizard28 added the safe-to-test Run CI tests for dependabot and external contributors label May 13, 2025
@grandwizard28 grandwizard28 merged commit dfca5b1 into SigNoz:main May 13, 2025
29 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
safe-to-test Run CI tests for dependabot and external contributors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants