OB-34960 Decorate k8s Node facets with custom processors #74
Conversation
This file contains hidden or 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
Add "Status" and "Roles" facets Rework the testing a bit to allow using jmespath to query specific fields of interest instead of manually creating json-like objects in go to exactly match the output.
|
I pushed a 2nd commit because I didn't know it would fall into the same PR, I thought it was gonna work the same as gerrit. If that's ok for you, we can leave it as-is and merge both together (it kinda makes sense as the second commit depends on the refactorings I made in the first one). |
obs-gh-enricogiorio
force-pushed
the
enrico/nodeFacets
branch
from
9bb3262 to
9f8ada5
Compare
components/processors/observek8sattributesprocessor/processor.go
Outdated
Show resolved
Hide resolved
Add a single custom processor that generates 3 facets: - restarts - ready_containers - total_containers The reasoning behind a single-processor for multiple facets is that all of these 3 facets are computed by iterating the same slice. Computing them in 3 different actions means iterating the same slice 3 times.
obs-gh-enricogiorio
force-pushed
the
enrico/nodeFacets
branch
2 times, most recently
from
49093a6 to
156db7d
Compare
| facetsMap.EnsureCapacity(facetsMap.Len() + len(values)) | ||
| for key, val := range values { | ||
| if err := mapPut(facetsMap, key, val); err != nil { | ||
| return logs, err |
There was a problem hiding this comment.
should we give up when we hit an error and return the logs in the current state? or should we just skip that action and continue
There was a problem hiding this comment.
Aha good catch. Let's continue instead of aborting the whole thing!
kubectl generates readinessGates as "0/1", "1/2", "2/2" (string). This may not be ideal for us, as we might want to do computation/aggregation on these columns separately. Therefore, we provide two (numeric) facets (from the same action): - readinessGatesReady - readinessGatesTotal
obs-gh-enricogiorio
force-pushed
the
enrico/nodeFacets
branch
from
156db7d to
bfb14e0
Compare
obs-gh-alexlew
approved these changes
Aug 15, 2024
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.
Add "Status" and "Roles" facets
Rework the testing a bit to allow using jmespath to query specific fields of interest instead of manually creating json-like objects in go to exactly match the output.
Description
This PR adds the "Roles" and "Status" facets to Node.
Since "Roles" is an array, I had to change the interface of the existing
ValueFn()to returnanyinstead ofstring, so that we can return maps and arrays, too.The current testing process is a bit cumbersome and doesn't scale well, since we are making a single
pcommon.Logscontaining all testing input logs, and then we have to maintain a parallel slice of "desired outputs", correlating inLogs[i] with outResults[i]. Since the correlation is always 1:1, I figured I could change this to:each testcase contains a single
pcommon.Logs, which contains only ONE Log. And each testcase also contains whatever verification(s) we want for the processed output of such log. Then we simply iterate over the testcases.I also got rid of all manually-created
map[string]interface{}json-like madness and usedjmespath, which allows for simpler and more powerful queries, so that we can selectively test what we need from the processed Logs!Given that we don't do correlation between logs, I figured that the assumption of always having a single log both as input and as output of the processor is enough for what we are doing here.
Checklist