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

OM-209 - latencies command is not working #130

Merged
merged 8 commits into from
Oct 30, 2024
Merged

OM-209 - latencies command is not working #130

merged 8 commits into from
Oct 30, 2024

Conversation

mphanias
Copy link
Contributor

this issue is occuring because namespace is append to the latencies command with a "-" and "-" (hypen) is part of the latency command itself, because of this command is constructed wrongly. a quick fix is use a separator which is not part of server command or output like "-","_",":" etc.,

this issue is occuring because namespace is append to the latencies command with a "-" and "-" (hypen) is part of the latency command itself, because of this command is constructed wrongly.
a quick fix is use a separator which is not part of server command or output like "-","_",":" etc.,
@mphanias mphanias requested a review from sunilvirus October 24, 2024 02:58
removed unnecessary code, and fixed service hist stats
removed unnecessary comments
as discussed, modified code to have different maps for namespace and service (exception) latency commands.
now latency commands are constructed and added while parsing the namespace and node-details
internal/pkg/statprocessors/statsprocessor.go Outdated Show resolved Hide resolved
internal/pkg/statprocessors/sp_latency.go Outdated Show resolved Hide resolved
internal/pkg/statprocessors/sp_latency.go Outdated Show resolved Hide resolved
@@ -5,7 +5,10 @@ var (
Service, ClusterName, Build string
)

var LatencyBenchmarks = make(map[string]string)
var NodeLatencyBenchmarks = make(map[string]string)
var NamespaceLatencyBenchmarks = make(map[string]string)
Copy link
Member

Choose a reason for hiding this comment

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

Though we do not parse map keys anymore, I still think map of maps is cleaner. any reason why we did not do this ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did that also, but with map of map, we have to do 2 loops, one good thing with the map of map will be the key will be namespace and looks more cleaner

internal/pkg/statprocessors/sp_latency.go Outdated Show resolved Hide resolved
internal/pkg/statprocessors/sp_latency.go Outdated Show resolved Hide resolved
Copy link
Member

@sunilvirus sunilvirus left a comment

Choose a reason for hiding this comment

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

LGTM. We can bring this in.

@sunilvirus sunilvirus merged commit da6a1b7 into dev Oct 30, 2024
1 check passed
@sunilvirus sunilvirus deleted the OM-209 branch October 30, 2024 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants