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

[BUG] Vega maps visualization doesn't use custom WMS server #934

Open
jcgraybill opened this issue Nov 12, 2021 · 6 comments
Open

[BUG] Vega maps visualization doesn't use custom WMS server #934

jcgraybill opened this issue Nov 12, 2021 · 6 comments
Assignees
Labels
bug Something isn't working maps Issues or PRs related to the Maps Service

Comments

@jcgraybill
Copy link

Describe the bug

The vega maps visualization only uses the default WMS tile server. If an admin overrides the default, that override is used for "region maps" visualizations but not vega maps visualizations.

To Reproduce
Add to opensearch_dashboards.yml

map.includeOpenSearchMapsService: false
map.tilemap.url: https://tiles.maps.opensearch.org/tiles/{z}/{x}/{y}.png
map.tilemap.options.attribution: "Im a map"

Create a new Vega visualization.

{
$schema: https://vega.github.io/schema/vega/v5.json
config: {
kibana: {type: "map"}
}
data: [
...

Expected behavior
Displayed map should use the tile server, attribution, etc from opensearch_dashboards.yml

OpenSearch Version
1.1.0

Dashboards Version
1.1.0

Plugins
Default installation

Additional context
https://github.com/opensearch-project/OpenSearch-Dashboards/tree/main/src/plugins/vis_type_vega/public/vega_view

@jcgraybill jcgraybill added bug Something isn't working untriaged labels Nov 12, 2021
@ahopp
Copy link
Contributor

ahopp commented Nov 16, 2021

@jcgraybill thanks for opening. Quick clarification: we're seeing an error "tmsServiceConfig is undefined" when trying to reproduce. Is this what you were seeing?

image

@tmarkley tmarkley added the maps Issues or PRs related to the Maps Service label Nov 16, 2021
@ahopp ahopp assigned ahopp and kavilla and unassigned kavilla Nov 16, 2021
@jcgraybill
Copy link
Author

I don't see that. See my screenshot...
image

@kavilla
Copy link
Member

kavilla commented Nov 23, 2021

I believe we might need to fork EMS Client. Will need to prioritize in that.

@jcgraybill
Copy link
Author

Any update?

@kavilla
Copy link
Member

kavilla commented Feb 8, 2022

Any update?

I have a private fork of the maps client that I'm current renaming.

AMoo-Miki pushed a commit to AMoo-Miki/OpenSearch-Dashboards that referenced this issue Feb 10, 2022
AMoo-Miki pushed a commit to AMoo-Miki/OpenSearch-Dashboards that referenced this issue Feb 10, 2022
# [24.3.0](elastic/elastic-charts@v24.2.0...v24.3.0) (2020-12-04)

### Bug Fixes

* **highlighter:** show default highlighted radius with hidden dots ([opensearch-project#926](elastic/elastic-charts#926)) ([351c20c](elastic/elastic-charts@351c20c)), closes [opensearch-project#679](elastic/elastic-charts#679)
* **xy_chart:** improve line joins rendering ([opensearch-project#920](elastic/elastic-charts#920)) ([9a6771c](elastic/elastic-charts@9a6771c))
* point highlight based on geom position and transform ([opensearch-project#934](elastic/elastic-charts#934)) ([196ee8d](elastic/elastic-charts@196ee8d))

### Features

* allow no results component, don't require series ([opensearch-project#936](elastic/elastic-charts#936)) ([6be5c8b](elastic/elastic-charts@6be5c8b))
* improved domain error handling ([opensearch-project#933](elastic/elastic-charts#933)) ([f480054](elastic/elastic-charts@f480054))
@kavilla
Copy link
Member

kavilla commented Mar 21, 2022

Followup, this is coming from the maps client but with forking there exist the issue that could change the expected behavior of the client.

Details:

It's failing because it can't find information within the provided TMS configurations provided. Within the client there exists this code that executes when includeOpenSearchMapsService is equal to false

 this._getMainCatalog = _.once(async (): Promise<OpenSearchMapsCatalogManifest> => {
      // Preserve manifestServiceUrl parameter for backwards compatibility with OpenSearchMaps v7.2
      if (this._manifestServiceUrl) {
        console.warn(`The "manifestServiceUrl" parameter is deprecated.
        Consider using "tileApiUrl" and "fileApiUrl" instead.`);
        return await this._getManifestWithParams(this._manifestServiceUrl);
      } else {
        const services = [];
        if (this._tileApiUrl) {
          services.push({
            type: 'tms',
            manifest: toAbsoluteUrl(this._tileApiUrl, `${this._opensearchMapsVersion}/manifest`),
          });
        }
        if (this._fileApiUrl) {
          services.push({
            type: 'file',
            manifest: toAbsoluteUrl(this._fileApiUrl, `${this._opensearchMapsVersion}/manifest`),
          });
        }
        return { services: services };
      }
    });

toAbsoluteUrl actually takes the value provided map.tilemap.url and tries to point it the the version of OpenSearch maps (which is defaulted to v7.10) + manifest. For example, in it's current stat it will try to make https://tiles.maps.opensearch.org/tiles/{z}/{x}/{y}.png to be https://tiles.maps.opensearch.org/v${mapsVersion}/manifest. Then the Maps Client attempts to get the manifest but fails since this does not exist. The actual value that this should be pointing to is https://maps.opensearch.org/tiles/v2.json. Then it would be able to get the value that OpenSearch Dashboards is expecting for example mapStyle being equal to road_map.

To work around this I was thinking add another configuration to allow for defining the manifest Url for tile maps and file maps, and I believe I was able to update the current maps visualizations but legacy map, ie map.tilemap, requires even more work.

Workaround proposal:

I think in the meantime to resolve this, I think if we can have if we can create a re-direct that points specifically https://tiles.maps.opensearch.org/v7.10/manifest to https://maps.opensearch.org/tiles/v2.json then using map.tilemap.url: https://tiles.maps.opensearch.org/tiles/{z}/{x}/{y}.png should work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working maps Issues or PRs related to the Maps Service
Projects
None yet
Development

No branches or pull requests

4 participants