DEVPROD-4507 Redact secrets from OTEL traces and Splunk logs for all graphql requests #7510
Conversation
graphql/redact_secrets_plugin.go
Outdated
|
|
||
| // RedactFieldsInMap recursively searches for and redacts fields in a map. | ||
| // Assumes map structure like map[string]interface{} where interface{} can be another map, a slice, or a basic datatype. | ||
| func RedactFieldsInMap(data map[string]interface{}) { |
There was a problem hiding this comment.
Can we write unit tests for this function?
I updated the description to explain why the second part is not possible. But I can certainly add a test for it. |
graphql/redact_secrets_plugin.go
Outdated
| } | ||
| err := util.DeepCopy(data, &dataCopy, registeredTypes) | ||
|
|
||
| if err != nil { |
There was a problem hiding this comment.
Nit: combine this with L85.
graphql/redact_secrets_plugin.go
Outdated
| } | ||
| } | ||
|
|
||
| _, err = file.WriteString("}\n\n") |
There was a problem hiding this comment.
I would have thought to have just one \n
| func isFieldRedacted(fieldName string, fieldsToRedact map[string]bool) bool { | ||
| _, ok := fieldsToRedact[fieldName] | ||
| return ok | ||
| } |
There was a problem hiding this comment.
is there an advantage to checking the field is in the map rather than just checking the value in the map? (since values not in the map will return false as well)
There was a problem hiding this comment.
I think there isn't an advantage to either since they both accomplish the same effect. Fields not in the map should be false either way.
| if e.settings.Tracer.CollectorAPIKey != "" { | ||
| opts = append(opts, otlptracegrpc.WithHeaders(map[string]string{ | ||
| honeycombCollectorHeader: e.settings.Tracer.CollectorAPIKey, | ||
| })) | ||
| } |
There was a problem hiding this comment.
maybe we'd also want to do this when useInternalDNS is true? On the other hand, maybe we'd never set the header when it's running in k8s. 🤷
…for all graphql requests (evergreen-ci#7510)" This reverts commit 7017279.
…nk logs for all graphql requests (evergreen-ci#7510)"" This reverts commit 2fcd87e.
DEVPROD-4507
Description
This PR does several things.
redactSecretswhich is used to flag input fields that contain secrets and we should avoid logging to places such as splunk or honeycomb.redactSecretsdirective. This is necessary because directives are not captured in otel traces. The trace spans begin after the directive middleware has already been run.REDACTED. This ensures it will work for all queries of all shapes since the query shape is determined by the client.otelgqlgentracer to intercept request variables and replace them.Some limitations
gqlgenlibrary's behavior, where it cannot determine the specific type of a field during query execution. Instead,gqlgenrecognizes the field as having amap[string]interface{}type. This interface represents a set of user-defined variables that do not align with the underlying GraphQL structure. Althoughgqlgenconducts some type evaluation to ensure compatibility, this process is not accessible in a manner that I could utilize. Consequently, the approach I adopted involves identifying the field names designated for redaction and masking any values corresponding to those names, irrespective of their origin within the GraphQL schema.Testing
Applied the directive to the field
taskNameSplunk logs
Documentation
remember to add or edit docs in the docs/ directory if relevant