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

service label to service_name & service_id labels #6

Merged
merged 3 commits into from
Oct 23, 2018

Conversation

mrnetops
Copy link
Contributor

Removes the service label and adds the service_name & service_id labels

  • Fastly Service IDs are immutable, while Service Names are not
  • Allows additional options for both querying and display

Copy link
Contributor

@peterbourgon peterbourgon left a comment

Choose a reason for hiding this comment

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

This is probably a worthwhile change, but I'm frankly a little surprised to see anyone actually cares about service ID. It's interesting to Fastly for diagnostic reasons, but what use cases exist for customers?

process.go Outdated
@@ -6,114 +6,114 @@ import (
"github.com/prometheus/client_golang/prometheus"
)

func process(src realtimeResponse, serviceName string, dst *prometheusMetrics) {
func process(src realtimeResponse, serviceName string, serviceID string, dst *prometheusMetrics) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Extremely pedantic nit, but can you put serviceID before serviceName in the parameter and label list? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm thinking of continuity of data. In cases where there are one or more service name changes over time, you either need to | each and every service name alternative over the duration or you get data presentation gaps. Alternately, if you graph by service IDs then you can freely and regularly rename services to your hearts content.

I'm happy to change the order, I just left it that way because you originally had serviceName as service first ;)

@peterbourgon
Copy link
Contributor

Also, if we're going to make this explicit, I suggest carrying that explictness forward thru the flag name and getServiceNames functionality. This patch should be able to be applied directly.

diff --git a/fastly.go b/fastly.go
index 8a59301..f37c113 100644
--- a/fastly.go
+++ b/fastly.go
@@ -12,19 +12,19 @@ import (
 	"github.com/go-kit/kit/log/level"
 )
 
-func getServiceNames(token string, services []string, logger log.Logger) map[string]string {
+func getServiceNames(token string, serviceIDs []string, logger log.Logger) map[string]string {
 	serviceNames := map[string]string{}
-	for _, service := range services {
-		serviceNames[service] = getServiceName(token, service, log.With(logger, "service", service))
+	for _, serviceID := range serviceIDs {
+		serviceNames[serviceID] = getServiceName(token, serviceID, log.With(logger, "service_id", serviceID))
 	}
 	return serviceNames
 }
 
-func getServiceName(token, service string, logger log.Logger) string {
-	req, err := http.NewRequest("GET", fmt.Sprintf("https://api.fastly.com/service/%s/details", url.QueryEscape(service)), nil)
+func getServiceName(token, serviceID string, logger log.Logger) string {
+	req, err := http.NewRequest("GET", fmt.Sprintf("https://api.fastly.com/service/%s/details", url.QueryEscape(serviceID)), nil)
 	if err != nil {
 		level.Error(logger).Log("err", err)
-		return service
+		return serviceID
 	}
 
 	req.Header.Set("Fastly-Key", token)
@@ -32,7 +32,7 @@ func getServiceName(token, service string, logger log.Logger) string {
 	resp, err := http.DefaultClient.Do(req)
 	if err != nil {
 		level.Error(logger).Log("err", err)
-		return service
+		return serviceID
 	}
 	defer resp.Body.Close()
 
@@ -41,11 +41,11 @@ func getServiceName(token, service string, logger log.Logger) string {
 	}
 	if err := json.NewDecoder(resp.Body).Decode(&response); err != nil {
 		level.Error(logger).Log("err", err)
-		return service
+		return serviceID
 	}
 	if response.Name == "" {
 		level.Error(logger).Log("err", "Fastly returned blank service name")
-		return service
+		return serviceID
 	}
 
 	return response.Name
diff --git a/main.go b/main.go
index dd9654f..6529ede 100644
--- a/main.go
+++ b/main.go
@@ -24,14 +24,14 @@ var version = "dev"
 func main() {
 	fs := flag.NewFlagSet("fastly-exporter", flag.ExitOnError)
 	var (
-		token     = fs.String("token", "", "Fastly API token")
-		services  = stringslice{}
-		addr      = fs.String("endpoint", "http://127.0.0.1:8080/metrics", "Prometheus /metrics endpoint")
-		namespace = fs.String("namespace", "", "Prometheus namespace")
-		subsystem = fs.String("subsystem", "", "Prometheus subsystem")
-		debug     = fs.Bool("debug", false, "log debug information")
+		token      = fs.String("token", "", "Fastly API token")
+		serviceIDs = stringslice{}
+		addr       = fs.String("endpoint", "http://127.0.0.1:8080/metrics", "Prometheus /metrics endpoint")
+		namespace  = fs.String("namespace", "", "Prometheus namespace")
+		subsystem  = fs.String("subsystem", "", "Prometheus subsystem")
+		debug      = fs.Bool("debug", false, "log debug information")
 	)
-	fs.Var(&services, "service", "Fastly service (repeatable)")
+	fs.Var(&serviceIDs, "service", "Fastly service ID (repeatable)")
 	fs.Usage = usageFor(fs, "fastly-exporter [flags]")
 	fs.Parse(os.Args[1:])
 
@@ -49,13 +49,13 @@ func main() {
 		level.Error(logger).Log("err", "-token is required")
 		os.Exit(1)
 	}
-	if len(services) <= 0 {
-		level.Error(logger).Log("err", "at least one -service is required")
+	if len(serviceIDs) <= 0 {
+		level.Error(logger).Log("err", "at least one -service ID is required")
 		os.Exit(1)
 	}
 
 	level.Debug(logger).Log("msg", "looking up service names")
-	serviceNames := getServiceNames(*token, services, log.With(logger, "query", "api.fastly.com"))
+	serviceNames := getServiceNames(*token, serviceIDs, log.With(logger, "query", "api.fastly.com"))
 	for service, name := range serviceNames {
 		level.Info(logger).Log("fastly_service", service, "name", name)
 	}

@peterbourgon
Copy link
Contributor

Cool! I need to add tests! :D

@peterbourgon peterbourgon merged commit 0f5e615 into fastly:master Oct 23, 2018
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.

2 participants