Fails to post new log message to elasticsearch

[ghost]

Not sure if this project uses Issues to report bugs, but...

With versions: • elasticsearch 5.0.1 • pino-elasticsearch@1.1.1

Elasticsearch TRACE: 2016-11-21T01:25:53Z
  -> POST http://localhost:9200/pino/log?consistency=one&op_type=create
  {
    "pid": 95928,
    "hostname": "mbp.local",
    "level": 30,
    "time": "2016-11-21T01:25:53.396Z",
    "msg": 1,
    "v": 1
  }
  <- 400
  {
    "error": {
      "root_cause": [
        {
          "type": "illegal_argument_exception",
          "reason": "request [/pino/log] contains unrecognized parameter: [consistency]"
        }
      ],
      "type": "illegal_argument_exception",
      "reason": "request [/pino/log] contains unrecognized parameter: [consistency]"
    },
    "status": 400
  }

[mcollina]

I did not test this into Elastic v5. Does it happens all the time?Can you please add steps to reproduce?

[ghost]

It works correctly with elasticsearch 2.4. However, the error occurs every time with version 5.

To reproduce:

1. Start elasticsearch with defaults on localhost.
2. Create a test.js file:

   var pino = require('pino');
   var log = pino();
   log.info(1);

3. Run node test.js | pino-elasticsearch -l trace

[mcollina]

@dadoonet do you know which is the best way to deal with this? Where are the main differences between the two versions?

[dadoonet]

It's related to https://github.com/elastic/elasticsearch/pull/19454 I believe: consistency parameter is not supported anymore. Do you really need to use that?

[mcollina]

@sitkb would you like to do the testing and send a PR?

[ghost]

I'm very new to node, pino, and elastic. However, by simply removing the consistency parameter there was some progress.

A new issue showed up and is due to elastic/elasticsearch#21535. At this point, I don't know how to update the code to change the operation from create to index. Also, it is unclear if this would break compatibility with version 2.4.

Thoughts?

[dadoonet]

Not sure what the usecase is here but just don't provide the op type parameter.

[mcollina]

@sitkb IMHO we might even split the flow in two functions, elastic 2.4 and elastic 5.

From a continuous integration point of view, we will need to test against both versions.

[dadoonet]

But why did you add those options in the first place?

[mcollina]

@dadoonet I don't remember. I'm ok in removing them, if the current tests pass straight away with Elastic v2.4.

[mcollina]

So, this needs to be overhauled to port it to Elastic v5 😢 .

[dadoonet]

So you mean it was needed for 2.4?

[mcollina]

I thought it was needed. consistency can be removed safely.

However, https://github.com/elastic/elasticsearch/issues/21535 is much more serious, and it will need to be assessed properly.

[dadoonet]

Indeed: https://www.elastic.co/guide/en/elasticsearch/reference/5.0/breaking_50_index_apis.html#_optype_create_without_an_id But why do you need to use op_create instead of using default behavior? Is it like an optimization?

[mcollina]

I'm using client.create(). It seems the "best" way to create a document from the docs. Should I use client.index() instead?

[dadoonet]

client.index() is perfectly fine. create() just throws an exception in case you try to index a document which already exists. But here you don't provide the id, so this will never happen.

[mcollina]

Fixed in v2.0.0.