vendor: OTEL v1.19.0 / v0.45.0, containerd v1.7.11 #46830
Merged
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
|
./cc @milas |
|
|
|
Wondering if the |
thaJeztah
changed the title
thaJeztah
force-pushed
the
vendor_containerd_1.7.9
branch
from
ee6faab
to
1a0e714
Compare
|
Here's the issue (from daemon logs):
|
|
Hm... related; I see PR's in BuildKit were not happy so far either; |
|
Discussing in our call; it looks like this may be related to the containerd and moby are on v1.17.0, otel switched to v1.21.0 |
|
Hmm.. so "go.opentelemetry.io/otel/semconv/v1.17.0/httpconv" from https://github.com/open-telemetry/opentelemetry-go/releases/tag/v1.17.0
And they should be moved somewhere to "contrib"; Related; |
|
Ugh, yeah, the While they're now in contrib, they're internal, so unusable. I believe they also need to get updated in containerd 😭 |
|
Yeah, was trying to find out what the replacement was for them. What's the use of moving them to another repository if that rep has them as internal? 🫠 |
thaJeztah
force-pushed
the
vendor_containerd_1.7.9
branch
from
bf4d0b0
to
864a736
Compare
thaJeztah
changed the title
thaJeztah
force-pushed
the
vendor_containerd_1.7.9
branch
from
864a736
to
8831c44
Compare
thaJeztah
force-pushed
the
vendor_containerd_1.7.9
branch
2 times, most recently
from
c698d56
to
e7f7181
Compare
thaJeztah
commented
Dec 8, 2023
cmd/dockerd/daemon.go
Outdated
| @@ -238,6 +240,17 @@ func (cli *DaemonCli) start(opts *daemonOptions) (err error) { | |||
|
|
|||
| setOTLPProtoDefault() | |||
| otel.SetTextMapPropagator(propagation.NewCompositeTextMapPropagator(propagation.TraceContext{}, propagation.Baggage{})) | |||
|
|
|||
| // Taken from https://github.com/moby/buildkit/blob/v0.12.4/util/tracing/detect/detect.go#L100-L107 | |||
| res, err := resource.Detect(context.Background(), serviceNameDetector{}) | |||
There was a problem hiding this comment.
Do we still need this, or is "resource.Default()" enough?
There was a problem hiding this comment.
It looks like resource.Default() is sufficient:
- It uses
fromEnv{}detector internally: https://github.com/open-telemetry/opentelemetry-go/blob/v1.19.0/sdk/resource/resource.go#L208 fromEnv{}detector readsOTEL_SERVICE_NAME: https://github.com/open-telemetry/opentelemetry-go/blob/v1.19.0/sdk/resource/env.go#L53
(fyi if BuildKit also switches to using resource.Default() in a future version, that also side-steps the whole SchemaURL issue 🤔)
There was a problem hiding this comment.
Thanks for looking! Let me push a commit to change that.
I'll keep changes as separate commit, but will squash things before merging (just keeping my own sanity a bit 😂)
thaJeztah
commented
Dec 8, 2023
| if resp.StatusCode >= http.StatusBadRequest { | ||
| code = codes.Error | ||
| } | ||
| span.SetStatus(code, "") |
There was a problem hiding this comment.
Do we need this part if we don't have the "httpconv.ClientRequest"? (disabled that part, per suggestion above)
There was a problem hiding this comment.
Maybe outside the scope of this PR, but worth exploring updating this logic to use http.Transport with a custom dialer since httputil.NewClientConn is deprecated. The otelhttp library could wrap the transport in that case and maybe simplify this.
There was a problem hiding this comment.
Actually could probably be even easier to just wrap clientconn.Do as an http.RoundTripper and could use https://pkg.go.dev/go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp#NewTransport. Not sure if that matches what the old functionality was doing.
There was a problem hiding this comment.
Playing around with that idea here #46920. I'm curious if that makes this clearer or cleaner.
There was a problem hiding this comment.
Agree with @dmcgowan re: refactoring the actual (non-OTel) logic here for a future release, at which point we can bring back OTel "for free" in the process.
That said, I think your simplified status handling here is fine
thaJeztah
force-pushed
the
vendor_containerd_1.7.9
branch
from
e7f7181
to
8546ba3
Compare
thaJeztah
changed the title
milas
approved these changes
Dec 11, 2023
| if resp.StatusCode >= http.StatusBadRequest { | ||
| code = codes.Error | ||
| } | ||
| span.SetStatus(code, "") |
There was a problem hiding this comment.
Agree with @dmcgowan re: refactoring the actual (non-OTel) logic here for a future release, at which point we can bring back OTel "for free" in the process.
That said, I think your simplified status handling here is fine
cmd/dockerd/daemon.go
Outdated
| @@ -238,6 +240,17 @@ func (cli *DaemonCli) start(opts *daemonOptions) (err error) { | |||
|
|
|||
| setOTLPProtoDefault() | |||
| otel.SetTextMapPropagator(propagation.NewCompositeTextMapPropagator(propagation.TraceContext{}, propagation.Baggage{})) | |||
|
|
|||
| // Taken from https://github.com/moby/buildkit/blob/v0.12.4/util/tracing/detect/detect.go#L100-L107 | |||
| res, err := resource.Detect(context.Background(), serviceNameDetector{}) | |||
There was a problem hiding this comment.
It looks like resource.Default() is sufficient:
- It uses
fromEnv{}detector internally: https://github.com/open-telemetry/opentelemetry-go/blob/v1.19.0/sdk/resource/resource.go#L208 fromEnv{}detector readsOTEL_SERVICE_NAME: https://github.com/open-telemetry/opentelemetry-go/blob/v1.19.0/sdk/resource/env.go#L53
(fyi if BuildKit also switches to using resource.Default() in a future version, that also side-steps the whole SchemaURL issue 🤔)
|
Running a container: Traces from Building a container: Traces from |
thaJeztah
force-pushed
the
vendor_containerd_1.7.9
branch
2 times, most recently
from
f33f45a
to
24febc6
Compare
dmcgowan
approved these changes
Dec 12, 2023
Upgrade to the latest OpenTelemetry libraries; this will unblock a lot of downstream projects in the ecosystem to upgrade, as some of the parts here were pre-1.0/unstable. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
full diff: containerd/containerd@v1.7.8...v1.7.9 Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
full diff: containerd/containerd@v1.7.9...v1.7.10 Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
full diff: containerd/containerd@v1.7.10...v1.7.11 Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
full diff: moby/buildkit@v0.12.4...3b6880d Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
… uses This commit switches our code to use semconv 1.21, which is the version matching the OTEL modules, as well as the containerd code. The BuildKit 0.12.x module currently uses an older version of the OTEL modules, and uses the semconv 0.17 schema. Mixing schema-versions is problematic, but we still want to consume BuildKit's "detect" package to wire-up other parts of OTEL. To align the versions in our code, this patch sets the BuildKit detect.Resource with the correct semconv version. It's worth noting that the BuildKit package has a custom "serviceNameDetector"; https://github.com/moby/buildkit/blob/v0.12.4/util/tracing/detect/detect.go#L153-L169 Whith is merged with OTEL's default resource: https://github.com/moby/buildkit/blob/v0.12.4/util/tracing/detect/detect.go#L100-L107 There's no need to duplicate that code, as OTEL's `resource.Default()` already provides this functionality: - It uses fromEnv{} detector internally: https://github.com/open-telemetry/opentelemetry-go/blob/v1.19.0/sdk/resource/resource.go#L208 - fromEnv{} detector reads OTEL_SERVICE_NAME: https://github.com/open-telemetry/opentelemetry-go/blob/v1.19.0/sdk/resource/env.go#L53 This patch also removes uses of the httpconv package, which is no longer included in semconv 1.21 and now an internal package. Removing the use of this package means that hijacked connections will not have the HTTP attributes on the Moby client span, which isn't ideal, but a limited loss that'd impact exec/attach. The span itself will still exist, it just won't the additional attributes that are added by that package. Alternatively, the httpconv call COULD remain - it will not error and will send syntactically valid spans but we would be mixing & matching semconv versions, so won't be compliant. Some parts of the httpconv package were preserved through a very minimal local implementation; a variant of `httpconv.ClientStatus(resp.StatusCode))` is added to set the span status (`span.SetStatus()`). The `httpconv` package has complex logic for this, but mostly drills down to HTTP status range (1xx/2xx/3xx/4xx/5xx) to determine if the status was successfull or non-successful (4xx/5xx). The additional logic it provided was to validate actual status-codes, and to convert "bogus" status codes in "success" ranges (1xx, 2xx) into an error. That code seemed over-reaching (and not accounting for potential future _valid_ status codes). Let's assume we only get valid status codes. - https://github.com/open-telemetry/opentelemetry-go/blob/v1.21.0/semconv/v1.17.0/httpconv/http.go#L85-L89 - https://github.com/open-telemetry/opentelemetry-go/blob/v1.21.0/semconv/internal/v2/http.go#L322-L330 - https://github.com/open-telemetry/opentelemetry-go/blob/v1.21.0/semconv/internal/v2/http.go#L356-L404 Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
force-pushed
the
vendor_containerd_1.7.9
branch
from
24febc6
to
4d2a324
Compare
|
Rebased, squashed the last few commits, and updated the commit message to describe what was one; https://github.com/moby/moby/compare/24febc67fb417702ab9b48f71241cf2014d201d1..4d2a324fce13e12e1adb5e75f3421c26565a7340 diff below in case the link above gets garbage-collected; diff --git a/client/hijack.go b/client/hijack.go
index 775df6428e..573fe157fb 100644
--- a/client/hijack.go
+++ b/client/hijack.go
@@ -65,7 +65,7 @@ func (cli *Client) setupHijackConn(req *http.Request, proto string) (_ net.Conn,
}
ctx, span := tp.Tracer("").Start(ctx, req.Method+" "+req.URL.Path, trace.WithSpanKind(trace.SpanKindClient))
- // FIXME(thaJeztah): what's the replacement for this in semconv v1.21?
+ // FIXME(thaJeztah): httpconv.ClientRequest is now an internal package; replace this with alternative for semconv v1.21
// span.SetAttributes(httpconv.ClientRequest(req)...)
defer func() {
if retErr != nil {
@@ -98,8 +98,19 @@ func (cli *Client) setupHijackConn(req *http.Request, proto string) (_ net.Conn,
// Server hijacks the connection, error 'connection closed' expected
resp, err := clientconn.Do(req)
if resp != nil {
- // Very simplified variant of "httpconv.ClientStatus(resp.StatusCode))"
+ // This is a simplified variant of "httpconv.ClientStatus(resp.StatusCode))";
//
+ // The main purpose of httpconv.ClientStatus() is to detect whether the
+ // status was successful (1xx, 2xx, 3xx) or non-successful (4xx/5xx).
+ //
+ // It also provides complex logic to *validate* status-codes against
+ // a hard-coded list meant to exclude "bogus" status codes in "success"
+ // ranges (1xx, 2xx) and convert them into an error status. That code
+ // seemed over-reaching (and not accounting for potential future valid
+ // status codes). We assume we only get valid status codes, and only
+ // look at status-code ranges.
+ //
+ // For reference, see:
// https://github.com/open-telemetry/opentelemetry-go/blob/v1.21.0/semconv/v1.17.0/httpconv/http.go#L85-L89
// https://github.com/open-telemetry/opentelemetry-go/blob/v1.21.0/semconv/internal/v2/http.go#L322-L330
// https://github.com/open-telemetry/opentelemetry-go/blob/v1.21.0/semconv/internal/v2/http.go#L356-L404
diff --git a/cmd/dockerd/daemon.go b/cmd/dockerd/daemon.go
index d80300b3ac..d356566c2f 100644
--- a/cmd/dockerd/daemon.go
+++ b/cmd/dockerd/daemon.go
@@ -240,6 +240,8 @@ func (cli *DaemonCli) start(opts *daemonOptions) (err error) {
setOTLPProtoDefault()
otel.SetTextMapPropagator(propagation.NewCompositeTextMapPropagator(propagation.TraceContext{}, propagation.Baggage{}))
+ // Override BuildKit's default Resource so that it matches the semconv
+ // version that is used in our code.
detect.Resource = resource.Default()
detect.Recorder = detect.NewTraceRecorder()
diff --git a/testutil/helpers.go b/testutil/helpers.go
index 711ef65f43..f31ad27944 100644
--- a/testutil/helpers.go
+++ b/testutil/helpers.go
@@ -34,7 +34,7 @@ func (d devZero) Read(p []byte) (n int, err error) {
var tracingOnce sync.Once
-// configureTracing sets up an OTLP tracing exporter for use in tests.
+// ConfigureTracing sets up an OTLP tracing exporter for use in tests.
func ConfigureTracing() func(context.Context) {
if os.Getenv("OTEL_EXPORTER_OTLP_ENDPOINT") == "" {
// No OTLP endpoint configured, so don't bother setting up tracing.
@@ -52,9 +52,7 @@ func ConfigureTracing() func(context.Context) {
tp = trace.NewTracerProvider(
trace.WithSpanProcessor(sp),
trace.WithSampler(trace.AlwaysSample()),
- trace.WithResource(resource.NewSchemaless(
- attribute.KeyValue{Key: semconv.ServiceNameKey, Value: attribute.StringValue("integration-test-client")},
- )),
+ trace.WithResource(resource.NewSchemaless(semconv.ServiceName("integration-test-client"))),
)
otel.SetTracerProvider(tp)
diff --git a/vendor.mod b/vendor.mod
index 24b9157f68..726772d61b 100644
--- a/vendor.mod
+++ b/vendor.mod
@@ -141,7 +141,7 @@ require (
github.com/containerd/stargz-snapshotter/estargz v0.14.3 // indirect
github.com/containerd/ttrpc v1.2.2 // indirect
github.com/containernetworking/cni v1.1.2 // indirect
- github.com/cyphar/filepath-securejoin v0.2.3 // indirect
+ github.com/cyphar/filepath-securejoin v0.2.4 // indirect
github.com/dimchansky/utfbom v1.1.1 // indirect
github.com/dustin/go-humanize v1.0.0 // indirect
github.com/felixge/httpsnoop v1.0.4 // indirect
diff --git a/vendor.sum b/vendor.sum
index 2b40aece20..dc31b7d6c6 100644
--- a/vendor.sum
+++ b/vendor.sum
@@ -374,8 +374,8 @@ github.com/creack/pty v1.1.11/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ
github.com/creack/pty v1.1.18 h1:n56/Zwd5o6whRC5PMGretI4IdRLlmBXYNjScPaBgsbY=
github.com/creack/pty v1.1.18/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4=
github.com/cyphar/filepath-securejoin v0.2.2/go.mod h1:FpkQEhXnPnOthhzymB7CGsFk2G9VLXONKD9G7QGMM+4=
-github.com/cyphar/filepath-securejoin v0.2.3 h1:YX6ebbZCZP7VkM3scTTokDgBL2TY741X51MTk3ycuNI=
-github.com/cyphar/filepath-securejoin v0.2.3/go.mod h1:aPGpWjXOXUn2NCNjFvBE6aRxGGx79pTxQpKOJNYHHl4=
+github.com/cyphar/filepath-securejoin v0.2.4 h1:Ugdm7cg7i6ZK6x3xDF1oEu1nfkyfH53EtKeQYTC3kyg=
+github.com/cyphar/filepath-securejoin v0.2.4/go.mod h1:aPGpWjXOXUn2NCNjFvBE6aRxGGx79pTxQpKOJNYHHl4=
github.com/davecgh/go-spew v0.0.0-20151105211317-5215b55f46b2/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
diff --git a/vendor/github.com/cyphar/filepath-securejoin/.travis.yml b/vendor/github.com/cyphar/filepath-securejoin/.travis.yml
deleted file mode 100644
index b94ff8cf92..0000000000
--- a/vendor/github.com/cyphar/filepath-securejoin/.travis.yml
+++ /dev/null
@@ -1,21 +0,0 @@
-# Copyright (C) 2017 SUSE LLC. All rights reserved.
-# Use of this source code is governed by a BSD-style
-# license that can be found in the LICENSE file.
-
-language: go
-go:
- - 1.13.x
- - 1.16.x
- - tip
-arch:
- - AMD64
- - ppc64le
-os:
- - linux
- - osx
-
-script:
- - go test -cover -v ./...
-
-notifications:
- email: false
diff --git a/vendor/github.com/cyphar/filepath-securejoin/README.md b/vendor/github.com/cyphar/filepath-securejoin/README.md
index 3624617c89..4eca0f2355 100644
--- a/vendor/github.com/cyphar/filepath-securejoin/README.md
+++ b/vendor/github.com/cyphar/filepath-securejoin/README.md
@@ -1,6 +1,6 @@
## `filepath-securejoin` ##
-[](https://travis-ci.org/cyphar/filepath-securejoin)
+[](https://github.com/cyphar/filepath-securejoin/actions/workflows/ci.yml)
An implementation of `SecureJoin`, a [candidate for inclusion in the Go
standard library][go#20126]. The purpose of this function is to be a "secure"
diff --git a/vendor/github.com/cyphar/filepath-securejoin/VERSION b/vendor/github.com/cyphar/filepath-securejoin/VERSION
index 7179039691..abd410582d 100644
--- a/vendor/github.com/cyphar/filepath-securejoin/VERSION
+++ b/vendor/github.com/cyphar/filepath-securejoin/VERSION
@@ -1 +1 @@
-0.2.3
+0.2.4
diff --git a/vendor/github.com/cyphar/filepath-securejoin/join.go b/vendor/github.com/cyphar/filepath-securejoin/join.go
index 7dd08dbbdf..aa32b85fb8 100644
--- a/vendor/github.com/cyphar/filepath-securejoin/join.go
+++ b/vendor/github.com/cyphar/filepath-securejoin/join.go
@@ -39,17 +39,27 @@ func IsNotExist(err error) bool {
// components in the returned string are not modified (in other words are not
// replaced with symlinks on the filesystem) after this function has returned.
// Such a symlink race is necessarily out-of-scope of SecureJoin.
+//
+// Volume names in unsafePath are always discarded, regardless if they are
+// provided via direct input or when evaluating symlinks. Therefore:
+//
+// "C:\Temp" + "D:\path\to\file.txt" results in "C:\Temp\path\to\file.txt"
func SecureJoinVFS(root, unsafePath string, vfs VFS) (string, error) {
// Use the os.* VFS implementation if none was specified.
if vfs == nil {
vfs = osVFS{}
}
+ unsafePath = filepath.FromSlash(unsafePath)
var path bytes.Buffer
n := 0
for unsafePath != "" {
if n > 255 {
- return "", &os.PathError{Op: "SecureJoin", Path: root + "/" + unsafePath, Err: syscall.ELOOP}
+ return "", &os.PathError{Op: "SecureJoin", Path: root + string(filepath.Separator) + unsafePath, Err: syscall.ELOOP}
+ }
+
+ if v := filepath.VolumeName(unsafePath); v != "" {
+ unsafePath = unsafePath[len(v):]
}
// Next path component, p.
diff --git a/vendor/modules.txt b/vendor/modules.txt
index 0cc505ac73..c535ddea8a 100644
--- a/vendor/modules.txt
+++ b/vendor/modules.txt
@@ -402,7 +402,7 @@ github.com/cpuguy83/tar2go
# github.com/creack/pty v1.1.18
## explicit; go 1.13
github.com/creack/pty
-# github.com/cyphar/filepath-securejoin v0.2.3
+# github.com/cyphar/filepath-securejoin v0.2.4
## explicit; go 1.13
github.com/cyphar/filepath-securejoin
# github.com/deckarep/golang-set/v2 v2.3.0 |
|
Windows never fails to deliver on flaky tests 🙊 |
thaJeztah
commented
Dec 12, 2023
| trace.WithResource(resource.NewSchemaless( | ||
| attribute.KeyValue{Key: semconv.ServiceNameKey, Value: attribute.StringValue("integration-test-client")}, | ||
| )), | ||
| trace.WithResource(resource.NewSchemaless(semconv.ServiceName("integration-test-client"))), |
There was a problem hiding this comment.
This change is not strictly needed, but while working on this code, I found the semconv.ServiceName utility, and thought it was slightly cleaner than to manually construct the attribute.KeyValue
sam-thibault
approved these changes
Dec 12, 2023
|
Let's bring this one in; I think we've had enough eyes and discussions around this one. |
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.
Resourceto be set externally buildkit#4472vendor: vendor: upgrade OpenTelemetry to v1.19.0 / v0.45.0
Upgrade to the latest OpenTelemetry libraries; this will unblock a lot of
downstream projects in the ecosystem to upgrade, as some of the parts here
were pre-1.0/unstable.
vendor: github.com/containerd/containerd v1.7.9
full diff: containerd/containerd@v1.7.8...v1.7.9
vendor: github.com/containerd/containerd v1.7.10
full diff: containerd/containerd@v1.7.9...v1.7.10
vendor: github.com/containerd/containerd v1.7.11
full diff: containerd/containerd@v1.7.10...v1.7.11
vendor: github.com/moby/buildkit v0.12.5-0.20231208203051-3b6880d2a00f
full diff: moby/buildkit@v0.12.4...3b6880d
update to go.opentelemetry.io/otel/semconv/v1.21.0, remove "httpconv" uses
This commit switches our code to use semconv 1.21, which is the version matching
the OTEL modules, as well as the containerd code.
The BuildKit 0.12.x module currently uses an older version of the OTEL modules,
and uses the semconv 0.17 schema. Mixing schema-versions is problematic, but
we still want to consume BuildKit's "detect" package to wire-up other parts
of OTEL.
To align the versions in our code, this patch sets the BuildKit detect.Resource
with the correct semconv version.
It's worth noting that the BuildKit package has a custom "serviceNameDetector";
https://github.com/moby/buildkit/blob/v0.12.4/util/tracing/detect/detect.go#L153-L169
Whith is merged with OTEL's default resource:
https://github.com/moby/buildkit/blob/v0.12.4/util/tracing/detect/detect.go#L100-L107
There's no need to duplicate that code, as OTEL's
resource.Default()alreadyprovides this functionality:
This patch also removes uses of the httpconv package, which is no longer included
in semconv 1.21 and now an internal package. Removing the use of this package
means that hijacked connections will not have the HTTP attributes on the Moby
client span, which isn't ideal, but a limited loss that'd impact exec/attach.
The span itself will still exist, it just won't the additional attributes that
are added by that package.
Alternatively, the httpconv call COULD remain - it will not error and will send
syntactically valid spans but we would be mixing & matching semconv versions,
so won't be compliant.
Some parts of the httpconv package were preserved through a very minimal local
implementation; a variant of
httpconv.ClientStatus(resp.StatusCode))is addedto set the span status (
span.SetStatus()). Thehttpconvpackage has complexlogic for this, but mostly drills down to HTTP status range (1xx/2xx/3xx/4xx/5xx)
to determine if the status was successfull or non-successful (4xx/5xx).
The additional logic it provided was to validate actual status-codes, and to
convert "bogus" status codes in "success" ranges (1xx, 2xx) into an error. That
code seemed over-reaching (and not accounting for potential future valid
status codes). Let's assume we only get valid status codes.
- What I did
- How I did it
- How to verify it
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)