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

Feature influx values (support for columns other than value) #4

Closed
wants to merge 2 commits into from

Conversation

breml
Copy link
Contributor

@breml breml commented Apr 25, 2014

I extendet the good work of @Dieterbe and @brutasse with the possibility to fetch not only the column value but every column. This is especial useful if one uses continuous queries where columns are named after the group by function (e.g. sum, mean).

brutasse and others added 2 commits April 11, 2014 21:48
With this extension all influxdb columns (beside of
time and sequence_number) will be available in Graphite.
This allows to use timeseries automaticaly calculated by
continuous queries, where the column name is named after
the group by function, e.g. sum, mean.

The column-name is separated by the constant string which is
configurable in the config-file. This string enables the possibitity
to identify leafs and therefore to shortcut in case of rendering a
graph. This may be replaced by caching in the finder.
@brutasse
Copy link
Contributor

Nice.

I used structlog in my modifications because that's what I usually use but I think standard logging is more what people will expect. To switch to standard logging, replace the import and the get_logger call with:

import logging
logger = logging.getLogger(__name__)

And then when logging messages, do logger.info(formatted_message), i.e. without passing keyword arguments. That's what you've done for the logging calls you added but mine would need to be modified.

For the record, here's a logging config for graphite-api that lets you see log messages:

logging:
  version: 1
  disable_existing_loggers: true
  handlers:
    stdout:
      class: logging.StreamHandler
  loggers:
    graphite_api:
      handlers:
        - stdout
      propagate: true
      level: DEBUG
    graphite_influxdb:
      handlers:
        - stdout
      propagate: true
      level: DEBUG

@Dieterbe
Copy link
Contributor

Dieterbe commented Jun 9, 2014

i applied the first commit, but with the 2nd enabled the code basically does:

          series = self.client.query("list series")
          for s in series:
              res = self.client.query('select * from "%s" limit 1' % s['name'])

this is slow and my gunicorn workers just timeout.
i have no conceptual objections against this idea, but the implementation should be faster.
ideally we would retrieve from influx all matching series (instead of regex filter in python), along with their columns ( to do your logic). all in one (potentially chunked) request, but no more than 1 http request should be necessary. related discussion about extending the list series feature @
influxdata/influxdb#376 )

also the 2nd patch would require a documentation update

@breml
Copy link
Contributor Author

breml commented Jun 11, 2014

Fine for me. I wasn't happy with the performance as well.
I was thinking about influxdata/influxdb#376 as well, but didn't find the time to actually try it out.

@Dieterbe
Copy link
Contributor

actually @breml to do this thing, we only need to get the list of columns right?
it seems any query like list queries or select * from /.*/ limit 1 will always include the colums for each series. so no need to query for each series individually.
I've been working on this area for the last couple of days. for now i use a cache and warm it up with the separate script, so that the graphite-api backend itself can just read from cache and generate the response quickly ( while we await influxdb improvements that will make querying for metadata much faster). So you could just change the current code a bit to not only store the series names, but also for each, their columns. and it should perform pretty well (assuming a warmed cache)

@Dieterbe
Copy link
Contributor

closing due to inactivity

@Dieterbe Dieterbe closed this Sep 16, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants