docker: adopt api_version as string to correct float limitations #30480
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
extension/observer
internal/docker
receiver/dockerstats
labels
rmfitzpatrick
force-pushed
the
dockerapiversion
branch
from
d397c50 to
865d364
Compare
atoulme
reviewed
Jan 12, 2024
atoulme
reviewed
Jan 12, 2024
crobert-1
reviewed
Jan 12, 2024
rmfitzpatrick
force-pushed
the
dockerapiversion
branch
2 times, most recently
from
251921a to
7702264
Compare
rmfitzpatrick
commented
Jan 12, 2024
| @@ -49,11 +49,8 @@ func (config Config) Validate() error { | |||
| if config.Endpoint == "" { | |||
| return errors.New("endpoint must be specified") | |||
| } | |||
| if config.CollectionInterval == 0 { | |||
There was a problem hiding this comment.
removed since this is a redundant check already covered by ScraperControllerSettings validation.
rmfitzpatrick
force-pushed
the
dockerapiversion
branch
2 times, most recently
from
eab5614 to
0c65d02
Compare
rmfitzpatrick
changed the title
rmfitzpatrick
force-pushed
the
dockerapiversion
branch
from
0c65d02 to
5272be0
Compare
dmitryax
reviewed
Jan 16, 2024
dmitryax
approved these changes
Jan 17, 2024
mfyuce
pushed a commit
to mfyuce/opentelemetry-collector-contrib
that referenced
this pull request
Jan 18, 2024
…n-telemetry#30480) **Description:** Fixing a bug - currently the docker client api version is a float, which is incompatible w/ the actual engine api version scheme (1.40 != 1.4). These changes adopt them as strings with validating helpers with minimum version checking using the docker types api*. Because [mapstructure's text unmarshaller hook](https://github.com/mitchellh/mapstructure/blob/63cde0dfe2481856bcfc2184477b26df770f19d7/decode_hooks.go#L266) only applies to strings I wasn't able to find a way to a complete fix without the breaking change of requiring api versions being strings (i.e. a user setting the value `1.40` will always be treated as a float 1.4)*. The proposed fix still suffers from the truncation problem but now provides the `!!str` option to offer a workaround that didn't exist. **Link to tracking Issue:** open-telemetry#24025
cparkins
pushed a commit
to AmadeusITGroup/opentelemetry-collector-contrib
that referenced
this pull request
Feb 1, 2024
…n-telemetry#30480) **Description:** Fixing a bug - currently the docker client api version is a float, which is incompatible w/ the actual engine api version scheme (1.40 != 1.4). These changes adopt them as strings with validating helpers with minimum version checking using the docker types api*. Because [mapstructure's text unmarshaller hook](https://github.com/mitchellh/mapstructure/blob/63cde0dfe2481856bcfc2184477b26df770f19d7/decode_hooks.go#L266) only applies to strings I wasn't able to find a way to a complete fix without the breaking change of requiring api versions being strings (i.e. a user setting the value `1.40` will always be treated as a float 1.4)*. The proposed fix still suffers from the truncation problem but now provides the `!!str` option to offer a workaround that didn't exist. **Link to tracking Issue:** open-telemetry#24025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
Fixing a bug - currently the docker client api version is a float, which is incompatible w/ the actual engine api version scheme (1.40 != 1.4). These changes adopt them as strings with validating helpers with minimum version checking using the docker types api*.
Because mapstructure's text unmarshaller hook only applies to strings I wasn't able to find a way to a complete fix without the breaking change of requiring api versions being strings (i.e. a user setting the value
1.40will always be treated as a float 1.4)*. The proposed fix still suffers from the truncation problem but now provides the!!stroption to offer a workaround that didn't exist.Link to tracking Issue:
#24025
Testing:
Added and updated tests where consumed
Documentation:
Updated readmes.