[exporter/elasticsearch] Fix dynamic mapping for double values storing integers

[carsonip]

Description:

Use json.Visitor SetExplicitRadixPoint option to ensure that double values are encoded like double values, such that ES determines the correct dynamic mapping for the field.

Link to tracking Issue:

Fixes #34680

Testing:

Updated tests

Documentation:

[andrzej-stencel]

I'm worried that this may potenially be a breaking change. Consider the following scenario:

A user uses the Elasticsearch exporter to export a metric that is defined as a double in OTLP, but that contains only integer values, into an Elasticsearch index that has the field mapped as an integer or long. After this change, the user will get an error from Elasticsearch, as the ES exporter will start sending values with the decimal point to Elasticsearch.

I think that at the very least, we should mark this as a breaking change, describing which users may be affected by this.

If we agree this is a breaking change, we need to also phase it in gradually, probably via a feature gate.

[carsonip]

Thanks for reviewing and thinking about the implication of this change. I understand your concern, but it should not be an issue.

but that contains only integer values, into an Elasticsearch index that has the field mapped as an integer or long. After this change, the user will get an error from Elasticsearch, as the ES exporter will start sending values with the decimal point to Elasticsearch.

I experimented with this. Sending double values to a field mapped as long will not result in any failures. They will be stored as a double in _source, while it will be truncated to return a long in fields API. Therefore, I'd say that this is addressed by ES and this change would not be a breaking change due to this.

{
    "took": 2,
    "timed_out": false,
    "_shards": {
        "total": 1,
        "successful": 1,
        "skipped": 0,
        "failed": 0
    },
    "hits": {
        "total": {
            "value": 3,
            "relation": "eq"
        },
        "max_score": 1.0,
        "hits": [
            {
                "_index": ".ds-logs-foo-default-2024.08.30-000001",
                "_id": "2YRyo5EBmDIc-wF_g7hx",
                "_score": 1.0,
                "_source": {
                    "foo": 4.5,
                    "@timestamp": "2024-08-30T13:21:16.657225033Z"
                },
                "fields": {
                    "foo": [
                        4
                    ]
                }
            },
            {
                "_index": ".ds-logs-foo-default-2024.08.30-000001",
                "_id": "2IRxo5EBmDIc-wF_07hR",
                "_score": 1.0,
                "_source": {
                    "foo": 4.0,
                    "@timestamp": "2024-08-30T13:20:31.569660078Z"
                },
                "fields": {
                    "foo": [
                        4
                    ]
                }
            },
            {
                "_index": ".ds-logs-foo-default-2024.08.30-000001",
                "_id": "1IRxo5EBmDIc-wF_dLgs",
                "_score": 1.0,
                "_source": {
                    "foo": 4,
                    "@timestamp": "2024-08-30T13:20:06.687976753Z"
                },
                "fields": {
                    "foo": [
                        4
                    ]
                }
            }
        ]
    }
}

@felixbarny @axw are there any cases I missed that would make this change a breaking change?

[felixbarny]

I'd also expect ES to coerce the floats to integers/longs, therefore this shouldn't be a breaking change. Thanks @carsonip for testing that coercion works as expected. Also good catch @andrzej-stencel on the potential breaking change here.

[andrzej-stencel]

Oh in that case all good, thanks a lot @carsonip for verifying this.

Can you fix the CI checks and we should be good to go.

Generated code is out of date, please run "make genotelcontribcol" and commit the changes in this PR.

[carsonip]

Can you fix the CI checks and we should be good to go.

I think the previous main was broken. I've rebased (merge main) and let's see if it works