Skip to content
This repository has been archived by the owner on Dec 8, 2021. It is now read-only.

Add enhanced JSON datasource #320

Merged
merged 2 commits into from
Dec 10, 2018
Merged

Add enhanced JSON datasource #320

merged 2 commits into from
Dec 10, 2018

Conversation

andig
Copy link
Contributor

@andig andig commented Oct 23, 2018

This is a type-script written fork of the original SimpleJSON datasource.

Most notably, it includes the ability to pass additional parameters as part of the /query request to allow differentiated responses by backend adapters:

screen shot 2018-10-23 at 19 50 01

One example backend adapter is https://github.com/andig/gravo which accepts grafana json queries and extracts multiple metrics simultaneously from an underlying volkszaehler REST api. Additional JSON parameters are used to provide fine-grained control on how to use the REST api (in the screenshot above by retrieving pre-aggregated data). In that respect it provides similar functionality, though with less abstraction due to its generic nature, as the Prometheus datasource.

Upstream PRs for grafana/simple-json-datasource#100, grafana/simple-json-datasource#103, grafana/simple-json-datasource#104 have been applied.

@andig andig changed the title [wip] Add enhanced JSON datasource Add enhanced JSON datasource Oct 24, 2018
@andig
Copy link
Contributor Author

andig commented Oct 24, 2018

Removed wip label, asking for review and merge :)

@zak-b2c2
Copy link

I am definitely a big supporter of this!

@torkelo
Copy link
Member

torkelo commented Nov 7, 2018

Not sure we want to publish a duplicate of the simple-json data source. The refactor to typescript would be a great PR to the original repo. I know this will sound pretty terrible as we have been so unresponsive on PRs and issues for the original simple-json-datasource. I think if we refactor it to typescript we could potentially move it to a built-in core data source (and write a go backend part would make it support alerting).

@andig
Copy link
Contributor Author

andig commented Nov 9, 2018

Not sure we want to publish a duplicate of the simple-json data source.

To start with I have to say that I'm entirely astonished. This is why:

  1. This is not a duplicate. Instead, it contains additional functionalities as described in #100, #103 and #104.

  2. There is user demand for these features.

  3. The cleanup (and furthermore typescript rewrite) of the simple json data source started in #95.

    The refactor to typescript would be a great PR to the original repo. I know this will sound pretty terrible as we have been so unresponsive on PRs and issues for the original simple-json-datasource.

    I can only agree that it sounds terrible since as of today, just 4 months later, that PR has not even seen a comment.

  4. Regarding forking, in add google calendar annotation datasource #105 @daniellee wrote:

    I am also not really sure if we want this PR in SimpleJson so maybe it is best if you focus on your datasource.

    in Added annotations-plugin #112 @danielllee wrote:

    The whole point of simple-json-datasource is to be forked (and that it is simple and not so feature rich) so please feel free to create a not-so-simple-json-datasource.

    This came as a surprise as it was not obvious from the documentation. So we took the cues and put efforts into the fork. Update The discussion about forking was actually open as well: #104 and got this reponse:

    Once you get to the stage where you need an extra json field then it usually gets specific to what you are building and in my experience is time to fork this into a new plugin. I will double check with my team on what they think about this.

    We can publish https://github.com/simPod/grafana-json-datasource on Grafana.com if you want.

I think if we refactor it to typescript we could potentially move it to a built-in core data source (and write a go backend part would make it support alerting).

That sounds like a valuable addition.

I would appreciate an open discussion of how to take this forward but from my point of view it needs a thorough commitment from grafana side to return the investment of our time with responsiveness and openness to features. Without the features we need like the ability to push additional json data, and without the ability to make improvements in due time, this is of little value.

/cc @simPod

@simPod
Copy link
Contributor

simPod commented Nov 9, 2018

Ummm...

...

...

@andig summed it up very well but in short this is what happened:

  1. We try to contribute to original repo with no response from maintainers for almost two months even though we tried to ping using metions. Thus we consider the repo abandoned/not worth Grafana team's effort (so far it's fine, happens).
  2. We start working on a fork.
  3. After some time, we receive message from Grafana that we should probably fork, that the original datasource is rather meant as a template for more feature-rich forked versions and should stay as lean as possible. And that if we build that fork it can be pushlished.
  4. We put some effort into it then, rewrite to typescript, add some features so it can do things that are essential for users of APIs.
  5. We try to apply for publishing.
  6. Grafana says they don't want to publish it but rather extend the original plugin.

I'm not sure where to go from here. This looks like Grafana states the exact opposite each month :/

@briangann briangann self-assigned this Nov 16, 2018
@briangann
Copy link
Contributor

The easiest way forward is to publish this as a new plugin.

You would need to change the plugin "id" in src/plugin.json to "id": "simpod-json-datasource" to differentiate it from the original plugin (name conflict).

We can plan out how to integrate this version into Grafana core afterwards.

If you make these changes I'll work to push through the review and get it published.

Thanks!

Brian

@briangann briangann added the submission/new Submission is a new plugin label Nov 16, 2018
@andig
Copy link
Contributor Author

andig commented Nov 16, 2018

Thanks @briangann. The original id is grafana-simple-json-datasource, we have grafana-json-datasource, so no conflict. Do you still need this change?

I‘d also appreciate any suggestions how we can contribute making json a backend plugin AND allowing our requirements to run on top if not in the core.

@briangann
Copy link
Contributor

Our convention for plugin id is githubusername-awesomeproject-plugintype, updating the id to simpod-json-datasource would work best (or even simpod-ejson-datasource). The github repo name doesn't have to change, just the id.

(you can also give it a different icon/logo so it stands out as different, but not mandatory)

I'll spin up the plugin and see what it would take for backend plugin integration. The process isn't documented well, but it is possible to create one and included it with a non-core plugin.

repo.json Outdated
@@ -2432,6 +2432,18 @@
}
]
},
{
"id": "grafana-json-datasource",
Copy link
Contributor

Choose a reason for hiding this comment

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

this will need to be updated to match the new id also

@andig
Copy link
Contributor Author

andig commented Nov 16, 2018

Yes, will do once we have another fix in.

@briangann briangann added this to In progress in publishing pipeline Nov 19, 2018
@briangann briangann added the milestone/needs-changes Submission needs changes before it can be approved label Nov 19, 2018
@briangann briangann moved this from In progress to Waiting for PR Changes in publishing pipeline Nov 19, 2018
@andig
Copy link
Contributor Author

andig commented Dec 3, 2018

@briangann update- hope this works now.

Copy link
Contributor

@briangann briangann left a comment

Choose a reason for hiding this comment

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

LGTM!

@briangann briangann removed the milestone/needs-changes Submission needs changes before it can be approved label Dec 5, 2018
@briangann briangann moved this from Waiting for PR Changes to Post-Review in publishing pipeline Dec 5, 2018
@briangann briangann merged commit 58d117c into grafana:master Dec 10, 2018
@andig
Copy link
Contributor Author

andig commented Dec 10, 2018

Great, thank you :)

@briangann briangann moved this from Post-Review to Published in publishing pipeline Dec 10, 2018
@briangann
Copy link
Contributor

you can see the plugin here: https://grafana.com/plugins/simpod-json-datasource

also available with grafana-cli

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
submission/new Submission is a new plugin
Projects
No open projects
publishing pipeline
  
Published
Development

Successfully merging this pull request may close these issues.

None yet

5 participants