-
Notifications
You must be signed in to change notification settings - Fork 37
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
feat: improve Rest Collector #2740
Conversation
cmd/tools/rest/href_builder.go
Outdated
@@ -81,6 +88,9 @@ func (b *HrefBuilder) Build() string { | |||
if b.returnTimeout != nil { | |||
addArg(&href, "&return_timeout=", strconv.Itoa(*b.returnTimeout)) | |||
} | |||
if b.isIgnoreUnknownFieldsEnabled && util.IsPublicAPI(b.apiPath) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ignore_unknown_fields=true works for private CLI. Is there a good reason to disallow its use? Allowing it would make our templates a bit more version resilient
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I have added it for private CLI as well.
We decided not to add this, as it didn't make sense based on one of the discussions, which is also documented here.
#2164 (comment)
Before:
After.log
Difference
Percentage Change
Alloc: Alloc improvements are in range of ~6-8% |
This PR reduces bytesRx by ~27% when monitoring the |
if err != nil { | ||
return nil, err | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not needed as err
just hasn't been used.
@@ -677,7 +677,7 @@ func (r *RestPerf) PollData() (map[string]*matrix.Matrix, error) { | |||
|
|||
href := rest.NewHrefBuilder(). | |||
APIPath(dataQuery). | |||
Fields(r.Prop.Fields). | |||
Fields([]string{"*"}). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for restperf, we will always send *
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. It will change as part of next story.
No description provided.