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

Add support for Elasticsearch pipelines #49

Closed

Conversation

imdevin567
Copy link

This adds support for Elasticsearch pipelines to allow for transformation of documents. We are currently running this in production without any issues pushing to Elasticsearch 6.0.

@mwinter69
Copy link

This becomes obsolete when #43 is merged. Here we will allow to configure a URL for elastic search.

@imdevin567
Copy link
Author

I might be misunderstanding what #43 is doing exactly, but I don't see how this is resolved with that PR. #43 involves refactoring, but pipeline support still does not exist. The URL for Elasticsearch is currently configurable, it just can't support pipelines because there is no way to add a query parameter in the URL box.

@mwinter69
Copy link

With #43 you no longer have host, port and path (not allowing a query string) separated. You give the full URL in one field which can include a query string.

@imdevin567
Copy link
Author

Ok, I see where the confusion is. The issue with including a query string in the URL is a dynamic ID still needs to be added to the end of the URL before it is passed on to Elasticsearch. For example, let's assume the following configuration:

Host: https://my-elasticsearch
Port: 443
Key: /logstash/jenkins

Right now, I could theoretically change the key to /logstash/jenkins?pipeline=mypipeline. Except the full path for an Elasticsearch log message is /index/type/id. With the new configuration, I could set the URL to https://my-elasticsearch:443/logstash/jenkins?pipeline=mypipeline, but that would trigger the plugin to send the log to https://my-elasticsearch:443/logstash/jenkins?pipeline=mypipeline/generatedId.

@jakub-bochenski
Copy link

@imdevin567 sorry, I don't follow. Where does the generatedId come from?

@imdevin567
Copy link
Author

https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-update.html

This shows the convention for sending a document to ES. I'm not sure where the generatedId comes from in this particular case, but an ID is required to add a document. It's being generated somehow and appended to the end of the URL, which in my case is after the pipeline=mypipeline query param.

@mwinter69
Copy link

I can't see in your coding anywhere that you generate an ID. The coding you provide would generate a URL of the form
https://my-elasticsearch:443/logstash/jenkins?pipeline=mypipeline
This you can also enter in the URL field with the new configuration approach (which btw is now merged to master branch).
Also in the link you provide I don't see where there is stated that you need an ID.
In https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-index_.html it is described that an ID not necessary and can be created automatically. I assume we're not updating documents but only creating new ones.

@imdevin567
Copy link
Author

Interesting. I have screenshots showing the behavior I'm seeing. Maybe I'm wrong in the reasoning as to why it's happening.

screen shot 2018-01-31 at 3 15 22 pm

Assume the above is my configuration. This theoretically should work based on how the URL is constructed. As you mentioned, this would create a URL in the form of https://elasticsearch:443/es/build?pipeline=jenkins_central.

screen shot 2018-01-31 at 3 25 15 pm

This is what I see in Elasticsearch. Rather than using the query string in the URL, it sends it as part of the type. That led me to believe there was an ID appending to the URL, which would wind up with https://elasticsearch:443/es/build?pipeline=jenkins_central/theId. If an ID isn't appended, why would this be an issue?

@mwinter69
Copy link

Can you try the following
/es/build/?pipeline=jenkins_central
In the doc the posts are always with a terminating /

@mwinter69
Copy link

Maybe the problem is also that the current implementation treats the key as a path, probably resulting in escaping of the '?'. With the new way of configuring a complete URL it should work fine, as the url is parsed and the parameters should really end up as parameters.

@imdevin567
Copy link
Author

screen shot 2018-01-31 at 3 44 07 pm

No dice. The character escaping makes sense, though that's strange behavior. I would be interested to try it. I can check out the new branch and see if it fixes that issue.

@mwinter69
Copy link

Obsolete, covered by URL in ElasticSearch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants