Giter VIP home page Giter VIP logo

promr's Introduction

Hi there 👋

promr's People

Contributors

glenn-m avatar konradzdeb avatar tylerlittlefield avatar

Stargazers

 avatar  avatar

Watchers

 avatar  avatar  avatar  avatar

promr's Issues

Uninformative error messages on wrong timespan

Problem

The when running sample code:

promR/README.Rmd

Lines 32 to 34 in 13faf82

library(promR)
prom <- Prometheus$new(host = "http://demo.robustperception.io", port = 9090)
metrics_current <- prom$query(query = "go_goroutines", time = format(Sys.time(), "%Y-%m-%dT%H:%M:%SZ"))

the query would return data frame with 0 rows:

metrics_current <- prom$query(query = "go_goroutines", 
                              time = format(Sys.time(),  "%Y-%m-%dT%H:%M:%SZ"))
# Error in format_metrics_current_data(metrics) : 
#  Assertion on 'x' failed: Must have at least 1 rows, but has 0 rows.
metrics_current <- prom$query(query = "go_goroutines", 
                             time = format(Sys.time() - 10,  "%Y-%m-%dT%H:%M:%SZ"))
 # Error in format_metrics_current_data(metrics) : 
 #  Assertion on 'x' failed: Must have at least 1 rows, but has 0 rows.

for:

Sys.time()
# [1] "2019-04-01 07:22:00 BST"

the data would be produced for Sys.time() - 3600

metrics_current <- prom$query(query = "go_goroutines", 
                               time = format(Sys.time() - 3600,  "%Y-%m-%dT%H:%M:%SZ"))

It's unclear whether:

  • there is no data for the period corresponding to 07:22 but there is for an hour earlier (?) Sys.time() - 3600
  • the connection failed
  • query failed

Suggested resolution

For internet connection

If possible to solve through #11 if not consider running curl::has_internet() before query, return stop

if (!curl::has_internet()) stop("This machine is not connected to the Internet :)")

For query

Shall return something like (?):

The query completed but returned no data....


Potentially related to #11

Enhancement: Basic Auth

Hi All,

Firstly thanks for taking the time to support this project.

Would there be an appetite to pass basic auth creds? For instance if Prometheus is hosted in Openshift/OKD it will sits behind an auth proxy.

The thinking would be if someone were to pass a username/password then try and authenticate, if these aren't present then don't use them.

Any thoughts?

Thanks

Matt

Ensure value column is cast as a reasonable type

Currently, it seems the value column is being cast as a character type, meaning users have to cast to complex or numeric.

library(promR)
library(ggplot2)
prom <- Prometheus$new(host = "http://demo.robustperception.io", port = 9090)
metrics_instant <- prom$query(query = 'go_goroutines', time = as.POSIXct(Sys.time() - 60))
m <- ggplot(metrics_instant, aes(metrics_instant$job)) + 
  geom_bar(aes(weight=as.complex(metrics_instant$value))) +
  xlab("Prometheus Jobs") +
  ylab("Value")
plot(m)

Ideally this should be handled for the user as the value column will always be a numerical value.

However, some Prometheus metrics will return scientific notation when the value is large enough.

For example:
alertmanager_silences_query_duration_seconds_bucket{le="2.5"} 4.766468e+06

Is there any reason we shouldn't just cast everything to the complex type? Or would it be better to default to numeric unless complex is required? What's your opinion @konradzdeb?

`port` label is shadowed by the port parsed from the `instance`

Consider exporter that exposes following metrics:

tcp_queue{state="LISTEN", dir="rx", addr="0.0.0.0", port="6432"} 0
tcp_queue{state="LISTEN", dir="rx", addr="127.0.0.1", port="5432"} 0

This is what prom$rangeQuery(query = 'tcp_queue', ...)$port returns instead of expected values:

 [1] 12345 12345 12345 12345 12345 12345 12345 12345 12345 12345 12345 12345
[13] 12345 12345

where 12345 actually comes from instance="127.0.0.1:12345"

Provide as.xts and as.ts method

Background

It could be useful for objects returned by promR to extend the existing as.xts and as.ts methods.

Desired behaviour

library(promR)
prom <- Prometheus$new(host = "http://demo.robustperception.io", port = 9090)
# Timestamps can be in rfc3339 or unix format
metrics_instant <- prom$query(query = "go_goroutines", 
                              time = as.POSIXct(Sys.time() - 60))
metrics_instant_xts <- xts::as.xts(metrics_instant)
class(metrics_instant_xts)
# [1] "xts" "zoo"

Current behaviour

xts::as.xts(metrics_instant)

Error in as.POSIXlt.character(x, tz, ...): character string is not in a standard unambiguous format


Side notes

May be worth reflecting whether there is merit in:

  • Providing zoo::as.zoo. Properly implemented as.xts method should return an object usable by zoo compatible methods. From ?as.xts documentation:

    (...) The returned object will preserve all relevant attribute/slot data within itself, allowing for temporary conversion to use zoo and xts compatible methods. (...)

  • as.ts method is likely, not useful in the current implementation, where metrics objects return data for various categories, as visible in table(metrics_instant$job).

Metrics data formatting function

With respect to the object that is returned here:

promR/R/promR.R

Lines 134 to 142 in aad5701

r <-
httr::GET(paste0(c(host, ":", port, "/api/v1/targets/metadata"), collapse = ""),
query = params)
# Check for particular status codes in response
response_check(r)
target_metadata <- jsonlite::fromJSON(httr::content(r, as = "text", encoding = "utf-8"))
return(target_metadata$data)

Is the generated data frame of similar format that is handled by the function that formats metrics for ranges and instant query? If so a potential solution could be to devise one generic function that formats all data frame outputs returned by Prometheus, irrespectively of the query. If you haven’t started on that, I’m happy to progress this (assuming that this is the right approach, of course)

format_metrics_range_data <- function(x) {
checkmate::assert_data_frame(x = x,
min.rows = 1,
min.cols = 2)
within(data = x$metric,
expr = {
port = as.integer(gsub(
pattern = "(.*):(.*)",
replacement = "\\2",
x = instance
))
instance = gsub(pattern = "(.*):(.*)",
replacement = "\\1",
x = instance)
}) -> x_metrics
lapply(
X = x$values,
FUN = function(value_pair) {
setNames(object = as.data.frame(value_pair),
nm = c("timestamp", "value")) -> df_ts_val
}
) -> dfs_to_bind
i <- 1
Reduce(rbind,
lapply(
X = dfs_to_bind,
FUN = function(dfs) {
suppressWarnings(cbind(x_metrics[i, ], dfs)) -> x
i <<- i + 1
x
}
)) -> res
return(res)
}

Make column names consistent

Currently we have differing column names depending on query, we should really make it more consistent. Mainly the name column name.

e.g.

An instant query results in:

X__name__ instance job timestamp value port
go_goroutines demo.robustperception.io prometheus 1554235843.452 87 9090
go_goroutines demo.robustperception.io pushgateway 1554235843.452 40 9091
go_goroutines demo.robustperception.io alertmanager 1554235843.452 34 9093
go_goroutines demo.robustperception.io node 1554235843.452 8 9100

where as range query results in:

name instance job port timestamp value
go_goroutines demo.robustperception.io prometheus 9090 1554235303.644 85
go_goroutines demo.robustperception.io prometheus 9090 1554235313.644 85
go_goroutines demo.robustperception.io prometheus 9090 1554235323.644 85
go_goroutines demo.robustperception.io prometheus 9090 1554235333.644 85
go_goroutines demo.robustperception.io prometheus 9090 1554235343.644 85
go_goroutines demo.robustperception.io prometheus 9090 1554235353.644 85

tl;dr X__name__ should be name instead

Missing documentation

devtools::check() returns a warning about missing documentation

❯ checking for missing documentation entries ... WARNING
  Undocumented S4 classes:
    ‘Prometheus’
  All user-level objects in a package (including S4 classes and methods)
  should have documentation entries.
  See chapter ‘Writing R documentation files’ in the ‘Writing R
  Extensions’ manual.

Improve testing using httptest

httptest offers a dedicated environment for testing functions that wrap APIs. I reckon we should consider this instead of faffing around trying to mock/wrap httr::GET. It seems that a substantial effort was put in that package to facilitate the development of testing and vignette generation. We would, in effect, swap one dependency for another mockery -> httptest. It just feels cleaner to me as it was developed for that purpose and looks like a more mature development.

httptest makes it easy to write tests for code and packages that wrap web APIs. Testing code that communicates with remote servers can otherwise be painful: things like authentication, server state, and network flakiness can make testing seem too costly to bother with. The httptest package enables you to test all of the logic on the R sides of the API in your package without requiring access to the remote service.

FYI @glenn-m

Protect end-users from passing nonsensical timeout values?

On that

promR/R/promR.R

Lines 32 to 39 in d5c1de0

if (!is.null(time)) {
params <- c(params, time = time)
}
# If timeout is not provided it uses server default
if (!is.null(timeout)) {
params <- c(params, timeout = timeout)
}

I was now thinking, is this timout in seconds? If so maybe it's good to add checkmate::assertInt to avoid people passing -10 as is.null won't protect from not sure. Not sure how defensive1 you want that script to be and whether it's worth the effort...


1checkmate: Fast Argument Checks for Defensive R Programming

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.