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

[exporter/elasticsearch] Encode geo location attributes as geo_point in Elasticsearch #36565

Closed
carsonip opened this issue Nov 27, 2024 · 8 comments · Fixed by #36594
Closed
Labels

Comments

@carsonip
Copy link
Contributor

carsonip commented Nov 27, 2024

Component(s)

exporter/elasticsearch

Is your feature request related to a problem? Please describe.

Geo location attributes geo.location.lat and geo.location.lon (SemConv) are stored as separate attributes in ES.

Describe the solution you'd like

They should be mapped as geo_point.

Describe alternatives you've considered

No response

Additional context

No response

@carsonip carsonip added enhancement New feature or request needs triage New item requiring triage labels Nov 27, 2024
@carsonip
Copy link
Contributor Author

/label -needs-triage

Copy link
Contributor

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@github-actions github-actions bot removed the needs triage New item requiring triage label Nov 27, 2024
@carsonip
Copy link
Contributor Author

@felixbarny @gregkalapos

@felixbarny
Copy link
Contributor

should geo.location.lon and geo.location.lat be removed after creating a new geo.location from the two of them?

Hm, I think so, yes. We're not losing information by combining them.

WDYT about using the format Geopoint expressed as a Well-Known Text POINT with the format: "POINT(lon lat)" in https://www.elastic.co/guide/en/elasticsearch/reference/current/geo-point.html ?

An advantage of that could be that it's easier to auto-detect geo fields without having to rely on naming conventions. But that would also require SemConv to adopt that format. The disadvantage is that it's more heavy in terms of bandwidth and storage, especially for backends that don't have native support for geo points. They'd need to store it as a string rather than an array of double values.

So overall, I'd lean towards storing it similar to Well-Known-Text, but as a double array ([lon, lat]).

We could then either introduce a naming convention (*.geo.location) or introduce type hints in OTLP.

@carsonip
Copy link
Contributor Author

We're not losing information by combining them.

I haven't tried, but will querying for geo.location.lon return the same value after merging lon and lat into geo.location?

@carsonip
Copy link
Contributor Author

I haven't tried, but will querying for geo.location.lon return the same value after merging lon and lat into geo.location?

Answering my own question, the answer is yes.

{
        "_index": ".ds-logs-foo.otel-default-2024.11.29-000001",
        "_id": "tw3Dd5MBgsycazFvRM-K",
        "_score": 1,
        "_source": {
          "@timestamp": "2024-11-29T11:50:00.000Z",
          "attributes": {
            "geo.location": {
              "lat": 2.1999999694526196,
              "lon": 1.0999999847263098
            }
          },
          "data_stream": {
            "type": "logs"
          }
        },
        "fields": {
          "geo.location": [
            {
              "coordinates": [
                1.0999999847263098,
                2.1999999694526196
              ],
              "type": "Point"
            }
          ],
          "geo.location.lon": [
            1.1
          ],
          "geo.location.lat": [
            2.2
          ]
        }
      }

@gregkalapos
Copy link
Member

@felixbarny @gregkalapos

  • should geo.location.lon and geo.location.lat be removed after creating a new geo.location from the two of them?

Yeah +1 on not storing those - also nice to see, we still can query only one if needed.

In which layer do you mean? If we really want to introduce that to attributes, then that'd be a new data type, so I'm with Felix on this, that'd be very hard with little benefit. But the ES exporter is still free to ingest it in whatever format we feel best from https://www.elastic.co/guide/en/elasticsearch/reference/current/geo-point.html.

@carsonip
Copy link
Contributor Author

In which layer do you mean?

I've ended up implemented using [lon, lat] in #36594 . Felix made a good call about storage size.

shivanthzen pushed a commit to shivanthzen/opentelemetry-collector-contrib that referenced this issue Dec 5, 2024
…tion in OTel mode (open-telemetry#36594)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

In OTel mapping mode, merge *.geo.location.{lat,lon} to *.geo.location
such that they are stored as
[geo_point](https://www.elastic.co/guide/en/elasticsearch/reference/current/geo-point.html)
in Elasticsearch.

<!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Fixes open-telemetry#36565

<!--Describe what testing was performed and which tests were added.-->
#### Testing

<!--Describe the documentation added.-->
#### Documentation

<!--Please delete paragraphs that you did not use before submitting.-->

---------

Co-authored-by: Vishal Raj <vishal.raj@elastic.co>
ZenoCC-Peng pushed a commit to ZenoCC-Peng/opentelemetry-collector-contrib that referenced this issue Dec 6, 2024
…tion in OTel mode (open-telemetry#36594)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

In OTel mapping mode, merge *.geo.location.{lat,lon} to *.geo.location
such that they are stored as
[geo_point](https://www.elastic.co/guide/en/elasticsearch/reference/current/geo-point.html)
in Elasticsearch.

<!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Fixes open-telemetry#36565

<!--Describe what testing was performed and which tests were added.-->
#### Testing

<!--Describe the documentation added.-->
#### Documentation

<!--Please delete paragraphs that you did not use before submitting.-->

---------

Co-authored-by: Vishal Raj <vishal.raj@elastic.co>
sbylica-splunk pushed a commit to sbylica-splunk/opentelemetry-collector-contrib that referenced this issue Dec 17, 2024
…tion in OTel mode (open-telemetry#36594)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

In OTel mapping mode, merge *.geo.location.{lat,lon} to *.geo.location
such that they are stored as
[geo_point](https://www.elastic.co/guide/en/elasticsearch/reference/current/geo-point.html)
in Elasticsearch.

<!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Fixes open-telemetry#36565

<!--Describe what testing was performed and which tests were added.-->
#### Testing

<!--Describe the documentation added.-->
#### Documentation

<!--Please delete paragraphs that you did not use before submitting.-->

---------

Co-authored-by: Vishal Raj <vishal.raj@elastic.co>
AkhigbeEromo pushed a commit to sematext/opentelemetry-collector-contrib that referenced this issue Jan 13, 2025
…tion in OTel mode (open-telemetry#36594)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

In OTel mapping mode, merge *.geo.location.{lat,lon} to *.geo.location
such that they are stored as
[geo_point](https://www.elastic.co/guide/en/elasticsearch/reference/current/geo-point.html)
in Elasticsearch.

<!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Fixes open-telemetry#36565

<!--Describe what testing was performed and which tests were added.-->
#### Testing

<!--Describe the documentation added.-->
#### Documentation

<!--Please delete paragraphs that you did not use before submitting.-->

---------

Co-authored-by: Vishal Raj <vishal.raj@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants