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

Add JSON field support #55

Merged
merged 8 commits into from
Nov 30, 2022
Merged

Add JSON field support #55

merged 8 commits into from
Nov 30, 2022

Conversation

toddtreece
Copy link
Member

  • buffer in chunks and push chunks on the query interval
  • add JSON field type support
  • update form layouts

go.mod Outdated
go 1.16
go 1.19

replace github.com/grafana/grafana-plugin-sdk-go => /home/todd/grafana-plugin-sdk-go
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^^^. remove before commit

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed in 88e5496

src/plugin.json Outdated
@@ -37,7 +37,7 @@
"updated": "%TODAY%"
},
"dependencies": {
"grafanaDependency": ">=8.0.0",
"grafanaDependency": ">=9.2.0",
Copy link
Member

@ryantxu ryantxu Nov 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason to bump all the way to 9.2? with 8.5, i think all the relevant APIs are the same

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SecretInput is missing pre 9.1.x, so i set the dependency to 9.1.0 in 5105e8d. i was thinking that the first official release should target a recent 9.x version since we don't have a lot of bandwidth to support 8.x issues. thoughts?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds reasonable -- especially for cloud

return &MQTTDatasource{
Client: client,
channelPrefix: fmt.Sprintf("ds/%s/", uid),
channelPrefix: path.Join("ds", uid),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this always use /? on windows does it use \

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

path/filepath is OS aware, but path.Join always uses /

@ryantxu
Copy link
Member

ryantxu commented Nov 18, 2022

This looks good -- What do you think about copying:
https://github.com/grafana/grafana/blob/main/pkg/services/live/pipeline/json_to_frame.go#L102

to this plugin and using it directly? Things got stalled with this plugin since I had hoped to use that in grafana server, but I don't see us getting that super production ready anytime soon

fieldMap map[string]int
}

func (df *framer) next() error {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ryantxu i attempted to modify the framer you linked to fit this use case. is this kind of what you were thinking?

@@ -0,0 +1,22 @@
# default configuration created by the `mage watch` command.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mage watch was added in grafana/grafana-plugin-sdk-go#563

@toddtreece toddtreece requested a review from a team November 22, 2022 12:48
@toddtreece toddtreece self-assigned this Nov 22, 2022
@NiklasCi
Copy link
Contributor

Hey. First of all thanks for implementing this.
I tried out your PR and can give some feedback:

  • The connection is very unstable. Tested with two brokers on different machines but the stream is failing alot:
INFO [11-23|14:56:13] MQTT Connecting                          logger=plugin.grafana-mqtt-datasource
ERROR[11-23|14:56:28] Error running stream, re-establishing    logger=live.runstream path=15s/test/json error="error running stream: rpc error: code = Unavailable desc = error reading from server: EOF" wait=400ms
ERROR[11-23|14:56:28] Error running stream, re-establishing    logger=live.runstream path=15s/test/json error="plugin unavailable" wait=0s
ERROR[11-23|14:56:28] Error running stream, re-establishing    logger=live.runstream path=15s/test/json error="plugin unavailable" wait=0s
ERROR[11-23|14:56:28] Error running stream, re-establishing    logger=live.runstream path=15s/test/json error="plugin unavailable" wait=200ms
ERROR[11-23|14:56:28] Error running stream, re-establishing    logger=live.runstream path=15s/test/json error="plugin unavailable" wait=400ms
  • Got also errors where the length of the fields has a missmatch
ERROR[11-23|14:54:19] failed to send data frame                logger=plugin.grafana-mqtt-datasource error="frame has different field lengths, field 0 is len 14 but field 1 is len 7" path=15s/test/json
  • Support JSON time values (RF3339)
case jsoniter.StringValue:
    v := df.iterator.ReadString()
    time, err := time.Parse(time.RFC3339, v)
    if err != nil {
        df.addValue(data.FieldTypeNullableString, &v)
    } else {
	df.addValue(data.FieldTypeNullableTime, &time)
    }

I don't know what exactly triggers the two errors above, but with main i don't have these problems as it seems. Maybe i do something wrong ?

My test value:

{
    "timestamp": "2022-11-08T09:05:50.989644528Z",
    "available": true,
    "ownership": "",
    "attached": true,
    "state": "attach",
    "P": -65440,
    "Q": -1930,
    "PSet": -65229.046875,
    "QSet": -1985.50830078125,
    "PLim": [
        100000,
        100000,
        100000,
        100000
    ]
}

@toddtreece
Copy link
Member Author

The connection is very unstable. Tested with two brokers on different machines but the stream is failing alot:

are there any other errors in your logs? it kind of seems like the plugin might be panicking and restarting

errors where the length of the fields has a missmatch

@NiklasCi thanks for testing! i'll follow up with a fix and test case for the field length issue.

Support JSON time values (RF3339)

this one could be nice to support in a future version if others request it, but for now i think it's covered by the convert field type transformation:
image

@NiklasCi
Copy link
Contributor

are there any other errors in your logs? it kind of seems like the plugin might be panicking and restarting

Ok tested back. Only the field lenght issue is reproducible. So the first issue could be on my end.

this one could be nice to support in a future version if others request it, but for now i think it's covered by the convert field type transformation:

Ah didn't know about the transformer. So my suggestion is not needed.

@toddtreece
Copy link
Member Author

@NiklasCi great, thanks for checking!

@toddtreece
Copy link
Member Author

@NiklasCi i think i have resolved the issues with field length. let me know if that works for you

@NiklasCi
Copy link
Contributor

@NiklasCi i think i have resolved the issues with field length. let me know if that works for you

Works like a charm now. Even with big json files and arrays containing objects.
Thanks alot.

@nmarrs nmarrs added the enhancement New feature or request label Nov 29, 2022
@toddtreece toddtreece merged commit c11488f into main Nov 30, 2022
@toddtreece toddtreece deleted the toddtreece/nested-json branch November 30, 2022 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants