Add docker & swarm container metadata #1685
Conversation
pkg/metadata/host/host_test.go
Outdated
| @@ -18,6 +20,8 @@ import ( | |||
|
|
|||
| func TestGetPayload(t *testing.T) { | |||
| p := GetPayload("myhostname") | |||
| pj, _ := json.Marshal(p) | |||
| fmt.Println(string(pj)) | |||
There was a problem hiding this comment.
Is this just testing that the payload is marshalled correctly? Capturing the error and using assert.NoError() instead of letting it panic would give better output I think.
There was a problem hiding this comment.
oups good catch, this was for debugging purposes I'll remove it 🙈
|
@CharlyF mentioned that adding the swarm node role (manager/worker) may make more sense as container meta instead of a host tag (DataDog/dd-agent#3735). Is container meta something that already exists in the agent5? |
|
@devonboyer yes it already exists, check this DataDog/dd-agent#3557 for more details |
mfpierre
force-pushed
the
mfpierre/container-meta
branch
2 times, most recently
from
2d3151b
to
f3bf572
Compare
mfpierre
force-pushed
the
mfpierre/container-meta
branch
from
f3bf572
to
729a1a6
Compare
pkg/util/docker/metadata.go
Outdated
| log.Debugf("Unable to collect Docker host metadata: %s", err) | ||
| return metadata | ||
| } | ||
| ctx, cancel := context.WithTimeout(context.Background(), du.queryTimeout) |
There was a problem hiding this comment.
To avoid delaying the metadata generation too much, could we use a 1 second timeout for all requests in this method?
As for GetSwarmState, if we think it will be reused, we could change it to a method accepting a types.Info object, like we do for parseStorageStatsFromInfo. That way:
- we can unit-test the parsing
- we can retrieve the info with a one-second timeout for this specific use
- later we could extract other tags from it without multiplying requests
There was a problem hiding this comment.
both swarm and server version are in types.Info simplifying the code 👍
pkg/util/docker/metadata.go
Outdated
| metadata["docker_version"] = v.Version | ||
| i, err := du.cli.Info(ctx) | ||
| if err != nil { | ||
| return metadata |
There was a problem hiding this comment.
We might want to either log or return the error, if we need to debug it one day
What does this PR do?
Add a new
container-metafield in metadata payloadAdd
docker_versionanddocker_swarmto it as it was reported by agent 5Motivation
Additional Notes
Other container metadata provider should follow