bfi-de / ngsild-grafana-datasource

A Grafana datasource plugin for FIWARE/NGSI-LD context brokers
Apache License 2.0
4 stars 4 forks source link

Temporal data cannot be retrieved from a Scorpio context broker #2

Closed pierre-josselin closed 3 weeks ago

pierre-josselin commented 8 months ago

Temporal data cannot be retrieved from a Scorpio context broker.

This is because the plugin requests temporal data with the parameter "options=temporalValues" which is not correctly supported by Scorpio.

Would it be possible to have an option to disable temporalValues in order to work with Scorpio? Thank you

Image of scorpio used in my case: scorpiobroker/all-in-one-runner:spring-kafka-3.0.11

cnoelle commented 8 months ago

Hi, could you please clarify what you mean by "is not correctly supported"? Does that also apply to the more recent versions (4.x) of Scorpio? Looking at their issues list, it seems that there have been several updates to the handling of temporal requests over the last months. Removing the parameter is not such a big deal, but if I understand it correctly the result should then be returned in a different format (not the simplified temporal representation), so the plugin would have to handle that.

cnoelle commented 8 months ago

By the way, the recent version of the NGSI-LD spec replaces the option parameter by format, deprecating the former. It would be good to make this configurable in the plugin, too. https://www.etsi.org/deliver/etsi_gs/CIM/001_099/009/01.08.01_60/gs_CIM009v010801p.pdf

pierre-josselin commented 8 months ago

@cnoelle

Hi, thank you very much for your quick response.

I did some additional tests to provide you more informations, and here are the results:

Regarding Scorpio 3.0.11, it seems that the presence of the options=temporalValues parameter returns always an empty array:

image

image

image

Removing this options=temporalValues parameter allows to recover the data again, but in the normal format:

image

[
  {
    "id": "urn:ngsi-ld:WeatherObserved:860077e9-467a-4c85-9ae5-831236e1e896",
    "type": "WeatherObserved",
    "solarRadiation": [
      {
        "type": "Property",
        "value": 504,
        "instanceId": "urn:ngsi-ld:6d57cee2-2f2e-4a2b-9039-42a1a4b9f108",
        "observedAt": "2024-03-26T15:47:57.979Z"
      },
      {
        "type": "Property",
        "value": 504,
        "instanceId": "urn:ngsi-ld:df720068-97b3-478b-8d94-2d282fd60493",
        "observedAt": "2024-03-26T15:47:57.979Z"
      },
      ...
    ],
    "windSpeed": [
      {
        "type": "Property",
        "value": 100,
        "instanceId": "urn:ngsi-ld:f7734707-85e2-4b73-849f-2f2af654bec8",
        "observedAt": "2024-03-25T17:23:50.067Z"
      },
      {
        "type": "Property",
        "value": 100,
        "instanceId": "urn:ngsi-ld:09968d0a-9efa-4c7c-a270-c197ffd4543f",
        "observedAt": "2024-03-25T17:23:50.067Z"
      },
      ...
    ]
  }
]

Regarding Scorpio 4.1.15 (latest version), the options=temporalValues parameter seems to be ignored, the data is returned in standard format whether it is present or not:

image

Which produces a parsing error:

image

However, as you mentioned, it works with the parameter format=simplified:

image


One last note, if you decide to make the modifications, you should consider that with Scorpio (maybe also with other context brokers), arrays that contain only one element are converted into an object, for example:

Two or more elements:

[
  {
    "id": "urn:ngsi-ld:Vehicle:0001",
    "type": "Vehicle",
    "location": [
      {
        "type": "Point",
        "coordinates": [
          7.282971,
          51.135245
        ]
      },
      {
        "type": "Point",
        "coordinates": [
          2.56263,
          49.928594
        ]
      }
    ]
  }
]

One element:

[
  {
    "id": "urn:ngsi-ld:Vehicle:0001",
    "type": "Vehicle",
    "location": {
      "type": "Point",
      "coordinates": [
        2.56263,
        49.928594
      ]
    }
  }
]
cnoelle commented 8 months ago

Thank you for the detailed explanation, @pierre-josselin . I think it makes sense to add an option using the standard representation, but I cannot promise a timing at the moment. You are welcome to contribute a pull-request, though, if this is an option.

Surprising to see that the current Scorpio still has an issue with options=temporalValues, since a related issue has been closed in January: https://github.com/ScorpioBroker/ScorpioBroker/issues/490. But maybe it did not land in the last release for some reason. Or the fix did not work.

pierre-josselin commented 8 months ago

Unfortunately we do not have availability at the moment to do this, but we are considering this possibility. In the meantime, we will use the plugin with Orion-LD and Mintaka.

Scorpio 3.0.11 was released a year ago, and the issue you mention dates from the beginning of the year, so I think the bug was still present in this version! But it's fixed in 4

cnoelle commented 1 month ago

still seems to be an issue in Scorpio: https://github.com/ScorpioBroker/ScorpioBroker/issues/620

fbuedding commented 1 month ago

still seems to be an issue in Scorpio: ScorpioBroker/ScorpioBroker#620

Yes I'v noticed that while testing.

@cnoelle Are you interested in getting a temporal fix in? I looked at the code and think that changing the type EntityTemporal to

export type EntityTemporal<V = any> = Omit<{
    [attribute: string]: {
        type: "Property" | "Relationship"; // TODO other possible?
        values: Measurement[];
    } |  Property<V>[];
}, "id" | "type" | "@context"> & {
    id: string; // the entity id
    type: string; // entity type
    "@context": any;
}

combined with narrowing the type as needed with the following functions

function isMeasurementArray(x: any[]): x is Measurement[] {
    return x.every(i => isMeasurement(i));
}
function isMeasurement(x: any): x is Measurement {
    return x instanceof Array 
        && typeof x[0] === "number" 
        && typeof x[1] === "string";
}

function isPropertyArray<V = any>(x: any[]): x is Property<V>[] {
    return x.every(i => isProperty(i));
}
function isProperty<V = any>(x: any): x is Property<V> {
    return x instanceof Object
        && x.value !== undefined
        && x.value["@value"] !== undefined
        && x.type !== undefined
        && x.type === "Property";
}

function isValue<V>(x: any): x is Value<V> {
  return x !== undefined || (x !== undefined && x["@value"] !== undefined)
}

would do the trick.

Since observedAt is optional it may be need to error when none is present for temporal queries.

Minimal working example

EDIT: I just realized, that the result of the scorpio broker doesn't have to return "@value" field

EDIT 2: I just realized, that my type definition was wrong

cnoelle commented 4 weeks ago

@fbuedding : your proposal certainly looks interesting, and I think it would be useful to support the other format. Unfortunately, I will probably not be able to look into this before end of next week. Maybe you could fork the project for the time being? If this works out for you I'd be happy to review a pull request. One other thought: have you tried what happens if you replace options=temporalValues with the alternative format=temporalValues? Scorpio should accept both variants and I do not really expect a different result, but you never know. And the plugin now supports toggling between options and format in the datasource settings.

cnoelle commented 3 weeks ago

Available in 1.2.0 as a dataource setting: select temporal representation: extended

fbuedding commented 3 weeks ago

It works thanks a lot!