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

fix: restore disabled uint support by default #13164

Merged
merged 2 commits into from
May 2, 2023

Conversation

powersj
Copy link
Contributor

@powersj powersj commented Apr 27, 2023

No description provided.

@telegraf-tiger telegraf-tiger bot added the fix pr to fix corresponding bug label Apr 27, 2023
@powersj powersj added the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label Apr 27, 2023
Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

Thanks for finding this @powersj. One question though...

@@ -169,7 +169,7 @@ func (i *InfluxDB) Write(metrics []telegraf.Metric) error {
}

func (i *InfluxDB) udpClient(address *url.URL) (Client, error) {
serializer := &influx.Serializer{UintSupport: true}
serializer := &influx.Serializer{}
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be

Suggested change
serializer := &influx.Serializer{}
serializer := &influx.Serializer{i.InfluxUintSupport}

as we otherwise now always disable uint-support!? Same for the other instances below...

Copy link
Contributor Author

@powersj powersj Apr 28, 2023

Choose a reason for hiding this comment

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

agreed - I've pushed a change for the influxdb/influxdb.go and influxdb_v2/influxdb_v2.go. @srebhan can you look at the influxdb/udp.go?

@telegraf-tiger
Copy link
Contributor

Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

LGTM. @powersj according to my understanding of the code before #12920 uint-support was only enabled if explicitly set which is not the case for UDP. Furthermore, we always provide a serializer to this code-path...

@srebhan srebhan merged commit 80b8fe6 into influxdata:master May 2, 2023
@srebhan srebhan added this to the v1.27.0 milestone Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix pr to fix corresponding bug plugin/serializer ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants