[SLT-152] feat(opbot): descending traces timestamp support #3092
Conversation
WalkthroughThe changes enhance the Changes
Poem
Tip Announcements
Recent review detailsConfiguration used: .coderabbit.yaml Files selected for processing (1)
Additional context usedGitHub Check: codecov/patch
Additional comments not posted (1)
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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3092 +/- ##
===================================================
+ Coverage 25.09282% 33.67474% +8.58192%
===================================================
Files 225 148 -77
Lines 12390 6444 -5946
Branches 82 82
===================================================
- Hits 3109 2170 -939
+ Misses 9044 4139 -4905
+ Partials 237 135 -102
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Deploying sanguine-fe with
|
| Latest commit: |
dbdd179
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://05b08f9e.sanguine-fe.pages.dev |
| Branch Preview URL: | https://opbot-orders.sanguine-fe.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- contrib/opbot/botmd/commands.go (4 hunks)
Additional context used
GitHub Check: codecov/patch
contrib/opbot/botmd/commands.go
[warning] 59-60: contrib/opbot/botmd/commands.go#L59-L60
Added lines #L59 - L60 were not covered by tests
[warning] 70-70: contrib/opbot/botmd/commands.go#L70
Added line #L70 was not covered by tests
[warning] 112-115: contrib/opbot/botmd/commands.go#L112-L115
Added lines #L112 - L115 were not covered by tests
Additional comments not posted (1)
contrib/opbot/botmd/commands.go (1)
12-12: LGTM!The import statement for the
sortpackage is approved.
contrib/opbot/botmd/commands.go
Outdated
| tags := strings.ToLower(stripLinks(ctx.Request().Param("tags"))) | ||
| splitTags := strings.Split(tags, " ")[:2] |
There was a problem hiding this comment.
LGTM!
The code changes to standardize tag inputs and limit the number of tags are approved.
Consider adding test coverage.
The static analysis tool indicates that these lines are not covered by tests.
Do you want me to generate test cases for these lines or open a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 59-60: contrib/opbot/botmd/commands.go#L59-L60
Added lines #L59 - L60 were not covered by tests
contrib/opbot/botmd/commands.go
Outdated
| @@ -65,6 +66,8 @@ | |||
| return | |||
| } | |||
|
|
|||
| isDescending := strings.Contains(tags, "desc") || strings.Contains(tags, "dsc") || strings.Contains(tags, "d") | |||
There was a problem hiding this comment.
LGTM!
The code change to introduce the isDescending variable is approved.
Consider adding test coverage.
The static analysis tool indicates that this line is not covered by tests.
Do you want me to generate test cases for this line or open a GitHub issue to track this task?
contrib/opbot/botmd/commands.go
Outdated
| if isDescending { | ||
| sort.Slice(traceList, func(i, j int) bool { | ||
| return traceList[i].Timestamp.After(traceList[j].Timestamp) | ||
| }) | ||
| } |
There was a problem hiding this comment.
LGTM!
The code changes to sort the traceList in descending order are approved.
Consider adding test coverage.
The static analysis tool indicates that these lines are not covered by tests.
Do you want me to generate test cases for these lines or open a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 112-115: contrib/opbot/botmd/commands.go#L112-L115
Added lines #L112 - L115 were not covered by tests
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- contrib/opbot/botmd/commands.go (3 hunks)
Additional context used
GitHub Check: codecov/patch
contrib/opbot/botmd/commands.go
[warning] 54-54: contrib/opbot/botmd/commands.go#L54
Added line #L54 was not covered by tests
[warning] 109-113: contrib/opbot/botmd/commands.go#L109-L113
Added lines #L109 - L113 were not covered by tests
contrib/opbot/botmd/commands.go
Outdated
| @@ -50,7 +51,7 @@ | |||
| // TODO: add trace middleware. | |||
| func (b *Bot) traceCommand() *slacker.CommandDefinition { | |||
| return b.requiresSignoz(&slacker.CommandDefinition{ | |||
| Command: "trace <tags>", | |||
| Command: "trace <tags> <order>", | |||
There was a problem hiding this comment.
LGTM!
The code change to introduce the <order> parameter is approved.
Consider adding test coverage.
The static analysis tool indicates that this line is not covered by tests.
Do you want me to generate test cases for this line or open a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 54-54: contrib/opbot/botmd/commands.go#L54
Added line #L54 was not covered by tests
contrib/opbot/botmd/commands.go
Outdated
| @@ -105,6 +106,12 @@ | |||
| } | |||
| return | |||
| } | |||
| isDescending := ctx.Request().Param("order") == "desc" || ctx.Request().Param("order") == "d" | |||
There was a problem hiding this comment.
LGTM!
The code change to introduce the isDescending variable is approved.
Consider adding test coverage.
The static analysis tool indicates that this line is not covered by tests.
Do you want me to generate test cases for this line or open a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 109-113: contrib/opbot/botmd/commands.go#L109-L113
Added lines #L109 - L113 were not covered by tests
contrib/opbot/botmd/commands.go
Outdated
| if isDescending { | ||
| sort.Slice(traceList, func(i, j int) bool { | ||
| return traceList[i].Timestamp.After(traceList[j].Timestamp) | ||
| }) |
There was a problem hiding this comment.
LGTM!
The code changes to sort the traceList in descending order are approved.
Consider adding test coverage.
The static analysis tool indicates that these lines are not covered by tests.
Do you want me to generate test cases for these lines or open a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 109-113: contrib/opbot/botmd/commands.go#L109-L113
Added lines #L109 - L113 were not covered by tests
trajan0x
changed the title
[goreleaser]
Bundle ReportChanges will decrease total bundle size by 1 bytes (-0.0%) ⬇️. This is within the configured threshold ✅ Detailed changes
|
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- contrib/opbot/botmd/commands.go (3 hunks)
Additional context used
GitHub Check: codecov/patch
contrib/opbot/botmd/commands.go
[warning] 54-54: contrib/opbot/botmd/commands.go#L54
Added line #L54 was not covered by tests
[warning] 57-59: contrib/opbot/botmd/commands.go#L57-L59
Added lines #L57 - L59 were not covered by tests
[warning] 63-63: contrib/opbot/botmd/commands.go#L63
Added line #L63 was not covered by tests
[warning] 112-117: contrib/opbot/botmd/commands.go#L112-L117
Added lines #L112 - L117 were not covered by tests
Additional comments not posted (1)
contrib/opbot/botmd/commands.go (1)
57-59: LGTM!The code changes to add new examples for the
tracecommand are approved.Tools
GitHub Check: codecov/patch
[warning] 57-59: contrib/opbot/botmd/commands.go#L57-L59
Added lines #L57 - L59 were not covered by tests
| @@ -50,14 +51,16 @@ | |||
| // TODO: add trace middleware. | |||
| func (b *Bot) traceCommand() *slacker.CommandDefinition { | |||
| return b.requiresSignoz(&slacker.CommandDefinition{ | |||
| Command: "trace <tags>", | |||
| Command: "trace {tags} {order}", | |||
There was a problem hiding this comment.
LGTM, but consider adding test coverage.
The code change to introduce the {order} parameter is approved.
However, the static analysis tool indicates that this line is not covered by tests.
Do you want me to generate test cases for this line or open a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 54-54: contrib/opbot/botmd/commands.go#L54
Added line #L54 was not covered by tests
| }, | ||
| Handler: func(ctx *slacker.CommandContext) { | ||
| tags := stripLinks(ctx.Request().Param("tags")) | ||
| splitTags := strings.Split(tags, " ") | ||
| splitTags := strings.Split(tags, "@") |
There was a problem hiding this comment.
LGTM, but consider adding test coverage.
The code change to convert the tags parameter to lowercase is approved.
However, the static analysis tool indicates that this line is not covered by tests.
Do you want me to generate test cases for this line or open a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 63-63: contrib/opbot/botmd/commands.go#L63
Added line #L63 was not covered by tests
| order := strings.ToLower(ctx.Request().Param("order")) | ||
| isAscending := order == "a" || order == "asc" | ||
| if isAscending { | ||
| sort.Slice(traceList, func(i, j int) bool { | ||
| return traceList[i].Timestamp.Before(traceList[j].Timestamp) | ||
| }) |
There was a problem hiding this comment.
LGTM, but consider adding test coverage.
The code changes to introduce the isAscending variable and sort the traceList in ascending order are approved.
However, the static analysis tool indicates that these lines are not covered by tests.
Do you want me to generate test cases for these lines or open a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 112-117: contrib/opbot/botmd/commands.go#L112-L117
Added lines #L112 - L117 were not covered by tests
Description
A clear and concise description of the features you're adding in this pull request.
Additional context
Add any other context about the problem you're solving.
Metadata
Summary by CodeRabbit
New Features
Bug Fixes