Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

elasticsearch logger: change op_type to create to allow sending to Data Streams #400

Merged
merged 3 commits into from
Sep 20, 2023

Conversation

ruffy91
Copy link
Contributor

@ruffy91 ruffy91 commented Sep 18, 2023

Since 0.35 indexing to elasticsearch does no longer work if the destination is a Data Stream.
This is because Data Streams only allow "create" (https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-bulk.html)

This pull request changes the "index" to a "create" op_type which allows sending logs to a Data Stream again.
"index" should not be needed as we never update existing documents.

Data Streams only allow "create".
"index" should not be needed as we never update existing documents while ingesting logs.
@dmachard
Copy link
Owner

@arvchristos can you take a look to this PR ? any remarks ?

@arvchristos
Copy link
Contributor

The PR is totally on point. In the future, we need to have some integration tests to catch issues like this one (I did not consider data streams when I implemented the bulk approach). I need to think how to best do integration testing for the Opensearch/Elasticsearch outputs but this is definitely a contribution in the right direction.

@dmachard
Copy link
Owner

Thank for this quick feedback :)

@fabian testunit need to be updated, CI raised the following error:

    elasticsearch_test.go:81: 
        	Error Trace:	/home/runner/work/go-dnscollector/go-dnscollector/loggers/elasticsearch_test.go:81
        	Error:      	Not equal: 
        	            	expected: map[string]interface {}(map[string]interface {}{})
        	            	actual  : <nil>(<nil>)
        	Test:       	Test_ElasticSearchClient/flat-json
 

@ruffy91
Copy link
Contributor Author

ruffy91 commented Sep 19, 2023

I did not run the tests locally because it immediately worked for me after compiling. Now the test is changed too and passes.

@dmachard dmachard changed the title Change op_type to create to allow sending to Data Streams elasticsearch logger: change op_type to create to allow sending to Data Streams Sep 19, 2023
@dmachard
Copy link
Owner

LGTM
thanks

@dmachard dmachard merged commit 5382897 into dmachard:main Sep 20, 2023
63 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants