Add config PopulateFailedDocsSource to bulk indexer #236
Conversation
elastic-observability-automation
bot
added
the
safe-to-test
| // in BulkIndexerResponseStat.FailedDocs is populated with the source of the item, | ||
| // which includes the action line and the document line. | ||
| // For testing and debugging only. Use with caution. | ||
| PopulateFailedDocsSource bool |
There was a problem hiding this comment.
[to reviewer] any suggestions for a better name? is PopulateFailedDocsBody clearer?
There was a problem hiding this comment.
Source might sound too much like _source, which is only the document content while in reality we return both action line and document line.
There was a problem hiding this comment.
I initially assumed it was _source from the PR title, so I think it does sound a bit too much like that. Maybe PopulateFailedDocsInput?
| // For testing and debugging only. Use with caution as it may expose sensitive data. | ||
| // Any clients should relay this warning to users. |
There was a problem hiding this comment.
| // For testing and debugging only. Use with caution as it may expose sensitive data. | |
| // Any clients should relay this warning to users. | |
| // | |
| // WARNING: this is provided for testing and debugging only. | |
| // Use with caution as it may expose sensitive data; any clients | |
| // of go-docappender enabling this should relay this warning to | |
| // their users. Setting this will also add memory overhead. |
Couple of suggestions:
- make the warning a bit more prominent
- mention that this will also impact memory usage (further discouraging use, and avoiding surprises)
| // in BulkIndexerResponseStat.FailedDocs is populated with the source of the item, | ||
| // which includes the action line and the document line. | ||
| // For testing and debugging only. Use with caution. | ||
| PopulateFailedDocsSource bool |
There was a problem hiding this comment.
I initially assumed it was _source from the PR title, so I think it does sound a bit too much like that. Maybe PopulateFailedDocsInput?
| @@ -132,6 +139,8 @@ type BulkIndexerResponseItem struct { | |||
| Type string `json:"type"` | |||
| Reason string `json:"reason"` | |||
| } `json:"error,omitempty"` | |||
|
|
|||
| Source string `json:"source"` | |||
There was a problem hiding this comment.
| Source string `json:"source"` | |
| Source string `json:"-"` |
This doesn't come back from Elasticsearch, let's make that clear with json:"-"
| @@ -295,3 +297,77 @@ func TestItemRequireDataStream(t *testing.T) { | |||
| require.NoError(t, err) | |||
| require.Equal(t, int64(2), stat.Indexed) | |||
| } | |||
|
|
|||
| func TestPopulateFailedItemSource(t *testing.T) { | |||
There was a problem hiding this comment.
Should we add a negative check, or is that covered elsewhere? i.e. check that Source is not populated unless we set the new config
Add config PopulateFailedDocsSource to bulk indexer to opt-in to getting back the source document that caused a failure. Should be used for testing and debugging only.