[endpoint] - Add more info in audit log #86
Conversation
In order to increase the visibility we have in the audit logs for some of our most recent endeavors like: - Migrations to FIPS - Clients using Protobuf instead of JSON We need to surface the informations that is related to the specific of the queries. We could add this into metrics but we would lose out on the specifics of who is making those request. Which is the important part when trying to get people to migrate and know the overall state of the platform. This PR aims to rely on the facilities already present in the audit logs. Namely, the custom annotations that are set. We can already see in some case annotation being set on the slow queries >500ms. This would effectively do the same. This patch should be fairly easy to cherry pick in the future as well, if we decide to keep it around once the migration are done. Arguably some of this could be of interest for upstream. Probably the content type more than the TLS cipher. So if we decide to keep it around long term we should create the relevant issue and possibly PR that in. datadog:patch
nyodas
force-pushed
the
bob/audit/moreinfo
branch
from
a826cb2
to
437e51b
Compare
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
| if len(events) != len(test.expected) { | ||
| t.Fatalf("Unexpected amount of lines in audit log: %d", len(events)) | ||
| } | ||
| for i, _ := range test.expected { |
There was a problem hiding this comment.
🔵 Code Quality Violation
| for i, _ := range test.expected { | |
| for i := range test.expected { |
Unnecessary blank identifier (...read more)
In Go, when using range iterations or receiving values from channels, it is recommended to avoid assigning the iteration or received value to the blank identifier _. Instead, it is preferred to omit the assignment entirely.
Here's why it is best to use for range s {}, x = someMap[key], and <-ch instead of using the blank identifier _:
- Clarity and Readability: By omitting the assignment entirely, it makes the code more readable and self-explanatory. Using
_can introduce confusion and make it less clear what the purpose of the assignment is or if the value is discarded intentionally or accidentally. - Avoiding Variable Pollution: Using
_as an assignment can unnecessarily pollute the variable space. Although Go allows the use of the blank identifier_to disregard a value, it is a good practice to avoid introducing unnecessary variables, especially if they are never used. - Linting and static analysis: Some linting tools and static analyzers may flag the use of
varName = _as an indication of accidental assignment or failure to handle errors or returned values properly. Removing these assignments eliminates such warnings or false-positive detections.
For example, consider the following code snippets:
for _ = range aSlice {}
x, _ = something()
_ = <- aChannelfor range aSlice {}
x = something()
<-aChannelBoth snippets achieve the same result, but the second one that omits the assignments using _ is preferred for its simplicity, readability, and adherence to Go's best practices.
By using for range s {}, x = someMap[key], and <-ch instead of assigning to _, you can write cleaner and more readable Go code while avoiding unnecessary variable assignments and potential confusion.
| if len(event.Annotations) != len(test.expected[i].Annotations) { | ||
| t.Errorf("[%s] expected %d annotations, got %d", test.desc, len(test.expected[i].Annotations), len(event.Annotations)) | ||
| } | ||
| for y, _ := range event.Annotations { |
There was a problem hiding this comment.
🔵 Code Quality Violation
| for y, _ := range event.Annotations { | |
| for y := range event.Annotations { |
Unnecessary blank identifier (...read more)
In Go, when using range iterations or receiving values from channels, it is recommended to avoid assigning the iteration or received value to the blank identifier _. Instead, it is preferred to omit the assignment entirely.
Here's why it is best to use for range s {}, x = someMap[key], and <-ch instead of using the blank identifier _:
- Clarity and Readability: By omitting the assignment entirely, it makes the code more readable and self-explanatory. Using
_can introduce confusion and make it less clear what the purpose of the assignment is or if the value is discarded intentionally or accidentally. - Avoiding Variable Pollution: Using
_as an assignment can unnecessarily pollute the variable space. Although Go allows the use of the blank identifier_to disregard a value, it is a good practice to avoid introducing unnecessary variables, especially if they are never used. - Linting and static analysis: Some linting tools and static analyzers may flag the use of
varName = _as an indication of accidental assignment or failure to handle errors or returned values properly. Removing these assignments eliminates such warnings or false-positive detections.
For example, consider the following code snippets:
for _ = range aSlice {}
x, _ = something()
_ = <- aChannelfor range aSlice {}
x = something()
<-aChannelBoth snippets achieve the same result, but the second one that omits the assignments using _ is preferred for its simplicity, readability, and adherence to Go's best practices.
By using for range s {}, x = someMap[key], and <-ch instead of assigning to _, you can write cleaner and more readable Go code while avoiding unnecessary variable assignments and potential confusion.
Description will come as this get polished