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

Use standard json naming convention #389

Closed
stuartnelson3 opened this Issue Apr 13, 2014 · 8 comments

Comments

Projects
None yet
3 participants
@stuartnelson3
Copy link
Member

stuartnelson3 commented Apr 13, 2014

This would probably require changes to every project, but I think it'd be nice to use standard JSON naming for the api, so instead of

{Type: "matrix",
 Value: [
  Metric: {},
  Values: [{Value: 1, Timestamp:1234428}]
}

it would be

{type: "matrix",
 value: [
  metric: {},
  values: [{values: 1, timestamp:1234428}]
}

thoughts?

@juliusv

This comment has been minimized.

Copy link
Member

juliusv commented Apr 13, 2014

In general, 👍 and we should do this at some point, but copy from my email:

Hey,

Thanks. I agree that lower-case is the way to go in general, but as they say...

The problem is that we have a bunch of known and possibly unknown places using the existing JSON query API already, besides PromDash. [redacted]. We'll need to map those out and synchronize a bunch of stuff before rolling out an API change. Shouldn't be too hard, but it's not high enough on the priorities list right now to spend time on it. In general, it would be cool to think of a way to version APIs in the future. One possibility is putting the version in the path names, another could be using header-based version negotiation...

@brian-brazil

This comment has been minimized.

Copy link
Member

brian-brazil commented Dec 26, 2014

If we change this, we should also change the format to be more compact. Rather than having a map per sample, a two item array would require transmitting fewer bytes over the wire.

@juliusv

This comment has been minimized.

Copy link
Member

juliusv commented Jan 2, 2015

👍 - though I think compressed encodings should take care pretty well of maps per sample? Anyways, I'm for two-item arrays.

If we want to change this, we'd need to do it kind of now-ish. But from our side, it would require quite some careful change management to not break things in production.

@brian-brazil

This comment has been minimized.

Copy link
Member

brian-brazil commented Jan 2, 2015

I'd suggest adding a url parameter that selects the output format, defaulting to the old format. At some point we can flip the default, and ultimately delete the old format.

@juliusv

This comment has been minimized.

Copy link
Member

juliusv commented Jan 20, 2015

Actually it might be easier to change the query clients to support both formats for a while. Putting conditional switches into the nested JSON-marshaled types in the server would get complicated/inefficient.

This is a change which updates the format that the server produces:

https://github.com/prometheus/prometheus/compare/change-json?expand=1

For vectors, it looks like this:

{
   "version": 1,
   "type" : "vector",
   "value" : [
      {
         "timestamp" : 1421765411.045,
         "value" : "65.475000",
         "metric" : {
            "quantile" : "0.5",
            "instance" : "http://localhost:9090/metrics",
            "job" : "prometheus",
            "__name__" : "http_request_duration_microseconds",
            "handler" : "/static/",
            "method" : "get",
            "code" : "304"
         }
      },
      {
         "timestamp" : 1421765411.045,
         "value" : "5826.339000",
         "metric" : {
            "quantile" : "0.9",
            "instance" : "http://localhost:9090/metrics",
            "job" : "prometheus",
            "__name__" : "http_request_duration_microseconds",
            "handler" : "prometheus",
            "method" : "get",
            "code" : "200"
         }
      },
      /* ... */
   ]
}

For matrices, it looks like this:

{
   "version": 1,
   "type" : "matrix",
   "value" : [
      {
         "metric" : {
            "quantile" : "0.99",
            "instance" : "http://localhost:9090/metrics",
            "job" : "prometheus",
            "__name__" : "http_request_duration_microseconds",
            "handler" : "/static/",
            "method" : "get",
            "code" : "200"
         },
         "values" : [
            [
               1421765547.659,
               "29162.953000"
            ],
            [
               1421765548.659,
               "29162.953000"
            ],
            [
               1421765549.659,
               "29162.953000"
            ],
            /* ... */
         ]
      }
   ]
}

Does that look sane? I'll let it sit there in any case for a while because it needs some changes in the other components to be compatible with it.

@brian-brazil

This comment has been minimized.

Copy link
Member

brian-brazil commented Jan 20, 2015

That looks sane to me.

@juliusv

This comment has been minimized.

Copy link
Member

juliusv commented Jan 20, 2015

Updated to have a version number.

juliusv added a commit to prometheus-junkyard/promdash that referenced this issue Jan 26, 2015

Add support for new query result JSON format.
See prometheus/prometheus#389.

This is a transient change and the "||" switches will be removed after a
week or so, when no old-format Prometheus servers are in use anymore.

simonpasquier pushed a commit to simonpasquier/prometheus that referenced this issue Oct 12, 2017

@lock

This comment has been minimized.

Copy link

lock bot commented Mar 24, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Mar 24, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.