pinojs/pino-elasticsearch

Fails to post new log message to elasticsearch

Closed this issue · 17 comments

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
  }

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

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);
  1. Run node test.js | pino-elasticsearch -l trace

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

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

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

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?

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

@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.

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

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

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

So you mean it was needed for 2.4?

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

However, elastic/elasticsearch#21535 is much more serious, and it will need to be assessed properly.

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?

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

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.

Fixed in v2.0.0.