TheThingsNetwork / lorawan-stack

The Things Stack, an Open Source LoRaWAN Network Server
https://www.thethingsindustries.com/stack/
Apache License 2.0
962 stars 303 forks source link

Show join-request, uplink and errors in Console traffic view #1962

Closed johanstokking closed 4 years ago

johanstokking commented 4 years ago

Summary

Show raw and decoded payload in Console traffic view

Why do we need this?

What is already there? What do you see now?

The event payload in as.up.forward is available when opening the row in the Console.

What is missing? What do you want to see?

as.up.data.forward has fields frm_payload (bytes) and decoded_payload (object). I'd like to see the former as hex and the latter as a key/value object (note; this can be nested); in the row. If you open the row, show the payload again. Also show ids.dev_addr.

For as.up.join.forward show the ids.join_eui and ids.dev_eui in the row.

When there's an error, show the formatted error (message_format with attributes) in red or something highlighted. Refs #1967)

How do you propose to implement this?

React magic

Can you do this yourself and submit a Pull Request?

Can review

Please coordinate

industrialinternet commented 4 years ago

Thanks for adding this it's a "must have" It was one of the first things we noticed when evaluating v3 on AWS market. Sorry to ask, any idea when?

johanstokking commented 4 years ago

@industrialinternet thanks for showing your interest. It's set in the February milestone so our aim is to finish it this month. Please subscribe to this issue and watch this repository, at least for releases, by clicking Watch in the top, so you know when it lands in which release.

bafonins commented 4 years ago

@johanstokking

Consider as.up.forward event bodies:

I guess you mean as.up.data.forward here?

  1. Can we assume that the fields you mentioned are always there (if the event operation is successful) for each event type?
  2. What fields should be included in the events widget component (the one of the entity overview pages) ? Screenshot 2020-02-10 at 15 35 28
  3. How big the decoded_payload object can be? Asking because it might be truncated in the event ui.
johanstokking commented 4 years ago

I guess you mean as.up.data.forward here?

Ah indeed, we split them out in as.up.join.forward and as.up.data.forward, and all the downlink events that I mentioned are not forwarded (yet).

  1. Can we assume that the fields you mentioned are always there (if the event operation is successful) for each event type?

2. What fields should be included in the events widget component (the one of the entity overview pages) ?

In cases where we have less space you mean? I think for data uplink the decoded payload, and for join-accepts the DevEUI.

  1. How big the decoded_payload object can be? Asking because it might be truncated in the event ui.

In the row it's fine to truncate it. If you expand the row it should be displayed as JSON. It can become quite big actually, it's up to the device maker or application developer.

But most is like {"temperature": 21.5, "humidity": 62, "x": -1, "y": 5}

johanstokking commented 4 years ago

Updated the original comment here

willhelmx commented 4 years ago

Hi johan and others involved in this matter

Nice to hear that you guy´s have a "milstone" for february. As we see it evaluating the V3 console, much can be done here with i think simple means to make an outstanding product for dev. and administration

1) Possibility to send downlinks to entire Application or Node separatly 2) Be able to see uplink payload HEX and decoded in separate window 3) Some kind of hierarchy for Applications directory fx. Parent/child/Node

Even if we plan to use TTI connected to our own servers this would make an exxcellent addition for R&D and monitoring

Anyway - nice work so far guy´s :-)

BR /A

johanstokking commented 4 years ago

Order of importance;

  1. Hex display of ids.join_eui, ids.dev_eui and ids.dev_addr in join-requests, ids.dev_addr and uplink_message.frm_payload for uplink messages
  2. Formatted error messages (i.e. fill in the attributes)
  3. Show decoded payload JSON
bafonins commented 4 years ago

Just to clarify things.

  1. js.join.accept
  2. ns.up.join.forward
  3. ns.up.merge_metadata
  4. as.up.join.receive
  5. as.up.join.forward This has the following view in the Console:
join-flow-otaa

Note the order of identifiers: join_eui, dev_eui and dev_addr

The uplink flow goes as follows:

  1. ns.up.merge_metadata
  2. ns.up.data.forward
  3. as.up.data.receive
  4. as.up.data.forward

This has the following view in the Console:

uplink-flow-otaa

Note the order of identifiers: dev_addr and frm_payload. I think we can also show the dev_addr for the rest of the events in the uplink flow.

The problem I see here is that we do not really have much space, especially for the as.up.join.forward event in the join flow. Moreover, just values dont really give much information and adding labels (Dev Addr: ....) would leave with even less space.

or for failed join request (ns.up.join.drop):

Screenshot 2020-03-24 at 17 36 08

Note that we keep the original, unformatted error in the payload inspector. This might be helpful for debugging.

@johanstokking @kschiffer any suggestions here?

johanstokking commented 4 years ago

Great first steps!

Note the order of identifiers: join_eui, dev_eui and dev_addr

Few comments/questions:

  1. Can we have a column for dev_addr, and fill that for all the uplink messages and join accepts that occur?
  2. I would prepend the JoinEUI and DevEUI with a small text like JoinEUI and DevEUI so that users know which one is which
  3. Show the identifiers for every message that we know, so this might be for many rows the same identifiers (JS/NS/AS). This is because we add filtering for events later and in some clusters, not all components are available (like JS-only) and we still want to see identifiers

Note the order of identifiers: dev_addr and frm_payload.

Good, also here prepend with FRMPayload

I think we can also show the dev_addr for the rest of the events in the uplink flow.

Yep, that's point 1 above. I agree with that.

The problem I see here is that we do not really have much space, especially for the as.up.join.forward event in the join flow. Moreover, just values dont really give much information and adding labels (Dev Addr: ....) would leave with even less space.

I would rather turn the "forward data uplink message" text into an icon (or icons; AS + uplink + data), rather than removing information to keep the event text. We can ask @pierrephz to design icons if we agree. cc @kschiffer

Note that we keep the original, unformatted error in the payload inspector. This might be helpful for debugging.

Agreed on the errors

kschiffer commented 4 years ago

Couple of thoughts:

Can we have a column for dev_addr, and fill that for all the uplink messages and join accepts that occur?

Let's add that as a loose element in the Data column.

I would prepend the JoinEUI and DevEUI with a small text like JoinEUI and DevEUI so that users know which one is which

Yes. @bafonins , this is actually what I meant in slack. Sorry for not being clear enough there.

I would rather turn the "forward data uplink message" text into an icon (or icons; AS + uplink + data).

"A text says more than a thousand icons" 😅. I'd really want to keep the event type column as text. It's impossible to communicate it's content solely via icons.

Here are two screendesigns that show what I think is a viable solution for application and device layer.

Overview Copy Singe Application Copy

bafonins commented 4 years ago

In the data views of single entities (end devices, gateways) we can remove the Entity ID column, since it is redundant

👍 makes sense

We need more fine-grained icons for different events, especially here, events relating to the join procedure

Not only for the join flow. It would be nice to have a custom icon for MAC commands (ns.mac.*).

Some event type messages are (as far as I can tell) unnecessarily lengthy, e.g. receive uplinke data message, couldn't it just be receive uplink data ?

Agree, it is just a matter of renaming several error descriptions. It could be receive uplink data or receive uplink message. @johanstokking what do you think?

It would be awesome to pipe the frm_payload through the current payload format function and to display the result as a one-line JSON (see screendesigns).

We can display decoded_payload directly, right? The structure of ApplicationUp for uplink messages is:

{
  "uplink_message": {
    ...
    "frm_payload": "AQ==",
    "decoded_payload": {
      "led": "ON"
    }
    ...
}

I would prepend the JoinEUI and DevEUI with a small text like JoinEUI and DevEUI so that users know which one is which

Yes. @bafonins , this is actually what I meant in slack. Sorry for not being clear enough there.

👌

kschiffer commented 4 years ago

Not only for the join flow. It would be nice to have a custom icon for MAC commands (ns.mac.*).

Indeed.

We can display decoded_payload directly, right? The structure of ApplicationUp for uplink messages is:

Oh, of course 🤦‍♂

johanstokking commented 4 years ago
  • e.g. receive uplinke data message, couldn't it just be receive uplink data ?

Yes. But can we have an icon for "receive" vs "forward" vs "send" etc?

I'd really want to keep the event type column as text. It's impossible to communicate it's content solely via icons.

Not solely, but like you suggested too, we can replace some obvious things by icons, right?

Can we have a column for dev_addr, and fill that for all the uplink messages and join accepts that occur?

Let's add that as a loose element in the Data column.

The thing is that this is part of every upstream message, including device activations (where we can show the new DevAddr). So in your first design, the DevAddr is not aligned (it's on the right), because it's loose, I guess?

johanstokking commented 4 years ago

Other than that, these designs look really good and helpful

kschiffer commented 4 years ago

Yes. But can we have an icon for "receive" vs "forward" vs "send" etc?

I think we'd either have to do it all the way or not at all. Replacing only some things by icons would be more confusing, I think.

I still need to think about the best way to represent event types by items. From what I see, there are three layers which are: stack component, process or subject(s) and step (e.g. ns.up.data.forward). Since it's not really possible to translate all of this information into one icon, we need to either use multiple icons or always focus on one of the event type layers, like subject.

Using three icons might be a way, but then again, I wouldn't really want to replace the event type text, so it would not really help with the spacing problem, but would at least make the event entries easier to scan.

The thing is that this is part of every upstream message, including device activations (where we can show the new DevAddr). So in your first design, the DevAddr is not aligned (it's on the right), because it's loose, I guess?

Indeed. But we can simply put the device address always first as a convention. If we have a dedicated column, we would lose space for all other events that do not need to show the device address.

kschiffer commented 4 years ago

So my suggestion to keep this actionable:

@bafonins let me know if you need any other info or clarification to continue with this issue.

johanstokking commented 4 years ago

Using three icons might be a way, but then again, I wouldn't really want to replace the event type text, so it would not really help with the spacing problem, but would at least make the event entries easier to scan.

Yeah that would be the main objective.

While we’re at that, we might want to consider grouping by correlation ID too. That will service both spacing (vertical) easier scanning.

Indeed. But we can simply put the device address always first as a convention. If we have a dedicated column, we would lose space for all other events that do not need to show the device address.

OK

bafonins commented 4 years ago

Some updates:

  1. We dont show the Entity ID column for device, gateway and organization events. Only for applications. This helps saving some extra space, especially for devices.
  2. Error event:
Screenshot 2020-04-28 at 19 45 21
  1. We show decoded_payload if available as plain list of values. skipping any nested entries like arrays or objects (I will follow up with implementation that adds colors to payload values). We also show frm_payload as hex when available:
Screenshot 2020-04-28 at 19 45 57
  1. Here is an example of the join flow: (application event view)
Screenshot 2020-04-28 at 19 46 30

(device event view)

Screenshot 2020-04-28 at 19 46 49
  1. Show frm_payload in hex for AS downlink events. This could be helpful for people scheduling downlinks via the Console:
Screenshot 2020-04-28 at 20 18 04

@johanstokking Is there anything else? Are there any entries we can show for gateway events (for gs.up.receive, for example, we can show frm_payload f_cnt f_port)?

johanstokking commented 4 years ago

Great!

Few minor comments/questions;

For the GS upstream events specifically;

~@htdvisser please advice on the event payload matters; this is not covered by development guidelines.~

bafonins commented 4 years ago

@johanstokking

For AS upstream events, also include the DevAddr

So, for as.up.data.receive, as.up.data.forward ? What about as.down.data.receive and as.down.data.forward ? I guess we also want to show the same for ns.up.data.*.

Are showing the event names now?

No, we will show full event description as it is now.

I would use the LoRaWAN terms DevAddr and FRMPayload

You mean instead of Device Address and Frame Payload ?

The decoded payload is usually a flat object; {"temperature":21.5,"light":"on"} etc. If there's a nested value, I'm fine skipping that; i.e. {"nested":{...},"light":"on"}

According to the designs, we show only values. So, for {"temperature":21.5,"light":"on"} we will show [21.5, "on"]. Is this fine?

GS does decode the LoRaWAN identifiers (EUIs on join, DevAddr on uplink), which could be interesting to show in this stream for filtering

We can show the identifiers for gs.up.receive. For join request we can show the EUI's from:

  "payload": {
    "join_request_payload": {
      "join_eui": "...",
      "dev_eui": "..."
    }
  }

And show dev addr for uplinks from:

  "payload": {
    "mac_payload": {
      "f_hdr": {
        "dev_addr": "...",
      }
    }
  }
johanstokking commented 4 years ago

So, for as.up.data.receive, as.up.data.forward ? What about as.down.data.receive and as.down.data.forward ? I guess we also want to show the same for ns.up.data.*.

Yes, if the identifiers are in the payload, show them.

You mean instead of Device Address and Frame Payload ?

Yes

According to the designs, we show only values. So, for {"temperature":21.5,"light":"on"} we will show [21.5, "on"]. Is this fine?

No I think we need keys here. Lots of values are numeric so that becomes confusing. Also because this is a map, there's no fixed order (unless you'd sort the keys and take the values).

We can show the identifiers for gs.up.receive. For join request we can show the EUI's from:

Yes but we don't have them in the payload, right? This is the payload for example:

{
  "@type": "type.googleapis.com/ttn.lorawan.v3.UplinkMessage",
  "raw_payload": "QOYAACeAws8CQ+4LarGLXmIEFQ==",
  "settings": {
    "data_rate": {
      "lora": {
        "bandwidth": 125000,
        "spreading_factor": 7
      }
    },
    "coding_rate": "4/5",
    "frequency": "867900000",
    "timestamp": 2986005427,
    "time": "2020-04-29T07:57:06Z"
  },
  "rx_metadata": [
    {
      "gateway_ids": {
        "gateway_id": "kerlink-ifemtocell",
        "eui": "7276FF003903007D"
      },
      "time": "2020-04-29T07:57:06Z",
      "timestamp": 2986005427,
      "rssi": -28,
      "channel_rssi": -28,
      "snr": 8,
      "uplink_token": "CiAKHgoSa2VybGluay1pZmVtdG9jZWxsEghydv8AOQMAfRCzp+uPCw==",
      "channel_index": 4
    }
  ],
  "received_at": "2020-04-29T07:57:06.748570190Z",
  "correlation_ids": [
    "gs:conn:01E6VEV9V14WMAY1DW19BQQPMX",
    "gs:uplink:01E72F0YSWJAKBYBJDBXG4CJ4G"
  ]
}
bafonins commented 4 years ago

Yes, if the identifiers are in the payload, show them

We can the identifiers from the identifiers field of the event if payload is empty:

    "identifiers": [
      {
        "device_ids": {
          "device_id": "...",
          "application_ids": {
            "application_id": "...",
          },
          "dev_eui": "...",
          "join_eui": "...",
          "dev_addr": "...",
        },
      },
    ],

No I think we need keys here. Lots of values are numeric so that becomes confusing. Also because this is a map, there's no fixed order (unless you'd sort the keys and take the values).

👌

Yes but we don't have them in the payload, right? This is the payload for example:

Here is what I have for receive uplink message - gs.up.receive:

{
  "@type": "type.googleapis.com/ttn.lorawan.v3.UplinkMessage",
  "raw_payload": "AAEAUAEAy6BYiiAcAAujBAB5X7wJxJ0=",
  "payload": {
    "m_hdr": {},
    "mic": "vAnEnQ==",
    "join_request_payload": {
      "join_eui": "...",
      "dev_eui": "...",
      "dev_nonce": "..."
    }
  },
  "settings": {
    "data_rate": {
      "lora": {
        "bandwidth": 125000,
        "spreading_factor": 7
      }
    },
    "coding_rate": "4/5",
    "frequency": "868300000",
    "timestamp": 3115131027,
    "time": "2020-04-29T08:13:09.690629005Z"
  },
  "rx_metadata": [
    {
      "gateway_ids": {
        "gateway_id": "bafonins-ttig",
        "eui": "58A0CBFFFE8010D6"
      },
      "time": "2020-04-29T08:13:09.690629005Z",
      "timestamp": 3115131027,
      "rssi": -35,
      "channel_rssi": -35,
      "snr": 8.25,
      "uplink_token": "ChsKGQoNYmFmb25pbnMtdHRpZxIIWKDL//6AENYQk8G0zQs="
    }
  ],
  "received_at": "2020-04-29T08:13:09.450967669Z",
  "correlation_ids": [
    "gs:conn:01E7140GJKZ5BKHMC774RV4C11",
    "gs:uplink:01E72FYAYB272T9X90MJEZNEAX"
  ]
}
johanstokking commented 4 years ago

OK, yes, show them. Currently they can be nil, so account for that, but we can change GS to decode the payload if that didn't happen yet.

bafonins commented 4 years ago

@johanstokking

Join flow with eui's where join_request_payload is available:

Screenshot 2020-05-03 at 19 41 26

Uplink with decoded payload:

Screenshot 2020-05-03 at 19 29 59

Failed event:

Screenshot 2020-05-03 at 19 30 10

AS uplink/downlink events:

Screenshot 2020-05-03 at 19 34 02

Gateway join request event:

Screenshot 2020-05-03 at 19 36 51

Gateway uplink with mac_payload:

Screenshot 2020-05-03 at 19 37 28
johanstokking commented 4 years ago

Awesome

One more request; please add FPort before every occurrence of FRMPayload

bafonins commented 4 years ago

Closed by https://github.com/TheThingsNetwork/lorawan-stack/pull/2477