Refactor methods to pass time.Duration rather than strings

pkg/cloud/awsprovider.go

@@ -314,7 +314,7 @@ var regionToBillingRegionCode = map[string]string{
 var loadedAWSSecret bool = false
 var awsSecret *AWSAccessKey = nil
 
-func (aws *AWS) GetLocalStorageQuery(window, offset string, rate bool, used bool) string {
+func (aws *AWS) GetLocalStorageQuery(window, offset time.Duration, rate bool, used bool) string {
 	return ""
 }
 

pkg/cloud/azureprovider.go

@@ -1127,7 +1127,7 @@ func (az *Azure) PVPricing(pvk PVKey) (*PV, error) {
 	return pricing.PV, nil
 }
 
-func (az *Azure) GetLocalStorageQuery(window, offset string, rate bool, used bool) string {
+func (az *Azure) GetLocalStorageQuery(window, offset time.Duration, rate bool, used bool) string {
 	return ""
 }
 

pkg/cloud/customprovider.go

@@ -5,6 +5,7 @@ import (
 	"strconv"
 	"strings"
 	"sync"
+	"time"
 
 	"github.com/kubecost/cost-model/pkg/clustercache"
 	"github.com/kubecost/cost-model/pkg/env"
@@ -42,7 +43,7 @@ func (*CustomProvider) ClusterManagementPricing() (string, float64, error) {
 	return "", 0.0, nil
 }
 
-func (*CustomProvider) GetLocalStorageQuery(window, offset string, rate bool, used bool) string {
+func (*CustomProvider) GetLocalStorageQuery(window, offset time.Duration, rate bool, used bool) string {
 	return ""
 }
 

pkg/cloud/gcpprovider.go

@@ -3,6 +3,7 @@ package cloud
 import (
 	"context"
 	"fmt"
+	"github.com/kubecost/cost-model/pkg/util/timeutil"
 	"io"
 	"io/ioutil"
 	"math"
@@ -124,7 +125,7 @@ func gcpAllocationToOutOfClusterAllocation(gcpAlloc gcpAllocation) *OutOfCluster
 
 // GetLocalStorageQuery returns the cost of local storage for the given window. Setting rate=true
 // returns hourly spend. Setting used=true only tracks used storage, not total.
-func (gcp *GCP) GetLocalStorageQuery(window, offset string, rate bool, used bool) string {
+func (gcp *GCP) GetLocalStorageQuery(window, offset time.Duration, rate bool, used bool) string {
 	// TODO Set to the price for the appropriate storage class. It's not trivial to determine the local storage disk type
 	// See https://cloud.google.com/compute/disks-image-pricing#persistentdisk
 	localStorageCost := 0.04
@@ -134,10 +135,7 @@ func (gcp *GCP) GetLocalStorageQuery(window, offset string, rate bool, used bool
 		baseMetric = "container_fs_usage_bytes"
 	}
 
-	fmtOffset := ""
-	if offset != "" {
-		fmtOffset = fmt.Sprintf("offset %s", offset)
-	}
+	fmtOffset := timeutil.DurationToPromString(offset)
 
 	fmtCumulativeQuery := `sum(
 		sum_over_time(%s{device!="tmpfs", id="/"}[%s:1m]%s)
@@ -151,8 +149,9 @@ func (gcp *GCP) GetLocalStorageQuery(window, offset string, rate bool, used bool
 	if rate {
 		fmtQuery = fmtMonthlyQuery
 	}
+	fmtWindow := timeutil.DurationString(window)
 
-	return fmt.Sprintf(fmtQuery, baseMetric, window, fmtOffset, env.GetPromClusterLabel(), localStorageCost)
+	return fmt.Sprintf(fmtQuery, baseMetric, fmtWindow, fmtOffset, env.GetPromClusterLabel(), localStorageCost)
 }
 
 func (gcp *GCP) GetConfig() (*CustomPricing, error) {

pkg/cloud/provider.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 	"strings"
+	"time"
 
 	"k8s.io/klog"
 
@@ -232,7 +233,7 @@ type Provider interface {
 	UpdateConfigFromConfigMap(map[string]string) (*CustomPricing, error)
 	GetConfig() (*CustomPricing, error)
 	GetManagementPlatform() (string, error)
-	GetLocalStorageQuery(string, string, bool, bool) string
+	GetLocalStorageQuery(time.Duration, time.Duration, bool, bool) string
 	ExternalAllocations(string, string, []string, string, string, bool) ([]*OutOfClusterAllocation, error)
 	ApplyReservedInstancePricing(map[string]*Node)
 	ServiceAccountStatus() *ServiceAccountStatus

pkg/costmodel/aggregation.go

@@ -196,7 +196,7 @@ func GetTotalContainerCost(costData map[string]*CostData, rate string, cp cloud.
 	return totalContainerCost
 }
 
-func (a *Accesses) ComputeIdleCoefficient(costData map[string]*CostData, cli prometheusClient.Client, cp cloud.Provider, discount float64, customDiscount float64, windowString, offset string) (map[string]float64, error) {
+func (a *Accesses) ComputeIdleCoefficient(costData map[string]*CostData, cli prometheusClient.Client, cp cloud.Provider, discount float64, customDiscount float64, window, offset time.Duration) (map[string]float64, error) {
 	coefficients := make(map[string]float64)
 
 	profileName := "ComputeIdleCoefficient: ComputeClusterCosts"
@@ -204,12 +204,12 @@ func (a *Accesses) ComputeIdleCoefficient(costData map[string]*CostData, cli pro
 
 	var clusterCosts map[string]*ClusterCosts
 	var err error
-
-	key := fmt.Sprintf("%s:%s", windowString, offset)
+	fmtWindow, fmtOffset := timeutil.DurationOffsetStrings(window, offset)
+	key := fmt.Sprintf("%s:%s", fmtWindow, fmtOffset)
 	if data, valid := a.ClusterCostsCache.Get(key); valid {
 		clusterCosts = data.(map[string]*ClusterCosts)
 	} else {
-		clusterCosts, err = a.ComputeClusterCosts(cli, cp, windowString, offset, false)
+		clusterCosts, err = a.ComputeClusterCosts(cli, cp, window, offset, false)
 		if err != nil {
 			return nil, err
 		}
@@ -225,7 +225,7 @@ func (a *Accesses) ComputeIdleCoefficient(costData map[string]*CostData, cli pro
 		}
 
 		if costs.TotalCumulative == 0 {
-			return nil, fmt.Errorf("TotalCumulative cluster cost for cluster '%s' returned 0 over window '%s' offset '%s'", cid, windowString, offset)
+			return nil, fmt.Errorf("TotalCumulative cluster cost for cluster '%s' returned 0 over window '%s' offset '%s'", cid, fmtWindow, fmtOffset)
 		}
 
 		totalContainerCost := 0.0
@@ -1492,11 +1492,9 @@ func (a *Accesses) ComputeAggregateCostModel(promClient prometheusClient.Client,
 			}
 		}
 
-		// Convert to Prometheus-compatible strings
-		durStr, offStr := timeutil.DurationOffsetStrings(dur, off)
-
-		idleCoefficients, err = a.ComputeIdleCoefficient(costData, promClient, a.CloudProvider, discount, customDiscount, durStr, offStr)
+		idleCoefficients, err = a.ComputeIdleCoefficient(costData, promClient, a.CloudProvider, discount, customDiscount, dur, off)
 		if err != nil {
+			durStr, offStr := timeutil.DurationOffsetStrings(dur, off)
 			log.Errorf("ComputeAggregateCostModel: error computing idle coefficient: duration=%s, offset=%s, err=%s", durStr, offStr, err)
 			return nil, "", err
 		}
@@ -1744,10 +1742,16 @@ func (a *Accesses) warmAggregateCostModelCache() {
 	// for the given duration. Cache is intentionally set to expire (i.e. noExpireCache=false) so that
 	// if the default parameters change, the old cached defaults with eventually expire. Thus, the
 	// timing of the cache expiry/refresh is the only mechanism ensuring 100% cache warmth.
-	warmFunc := func(duration, durationHrs, offset string, cacheEfficiencyData bool) (error, error) {
+	warmFunc := func(duration, offset time.Duration, cacheEfficiencyData bool) (error, error) {
+		if a.ThanosClient != nil {
+			duration = thanos.OffsetDuration()
+			log.Infof("Setting Offset to %s", duration)
+		}
+		fmtDuration, fmtOffset := timeutil.DurationOffsetStrings(duration, offset)
+		durationHrs, err := timeutil.DayDurationToHourDuration(fmtDuration)
 		promClient := a.GetPrometheusClient(true)
 
-		windowStr := fmt.Sprintf("%s offset %s", duration, offset)
+		windowStr := fmt.Sprintf("%s fmtOffset %s", fmtDuration, fmtOffset)
 		window, err := kubecost.ParseWindowUTC(windowStr)
 		if err != nil {
 			return nil, fmt.Errorf("invalid window from window string: %s", windowStr)
@@ -1778,17 +1782,15 @@ func (a *Accesses) warmAggregateCostModelCache() {
 
 		aggKey := GenerateAggKey(window, field, subfields, aggOpts)
 		log.Infof("aggregation: cache warming defaults: %s", aggKey)
-		key := fmt.Sprintf("%s:%s", durationHrs, offset)
+		key := fmt.Sprintf("%s:%s", durationHrs, fmtOffset)
 
 		_, _, aggErr := a.ComputeAggregateCostModel(promClient, window, field, subfields, aggOpts)
 		if aggErr != nil {
 			log.Infof("Error building cache %s: %s", window, aggErr)
 		}
-		if a.ThanosClient != nil {
-			offset = thanos.Offset()
-			log.Infof("Setting offset to %s", offset)
-		}
-		totals, err := a.ComputeClusterCosts(promClient, a.CloudProvider, durationHrs, offset, cacheEfficiencyData)
+
+
+		totals, err := a.ComputeClusterCosts(promClient, a.CloudProvider, duration, offset, cacheEfficiencyData)
 		if err != nil {
 			log.Infof("Error building cluster costs cache %s", key)
 		}
@@ -1800,9 +1802,9 @@ func (a *Accesses) warmAggregateCostModelCache() {
 		}
 		if len(totals) > 0 && maxMinutesWithData > clusterCostsCacheMinutes {
 			a.ClusterCostsCache.Set(key, totals, a.GetCacheExpiration(window.Duration()))
-			log.Infof("caching %s cluster costs for %s", duration, a.GetCacheExpiration(window.Duration()))
+			log.Infof("caching %s cluster costs for %s", fmtDuration, a.GetCacheExpiration(window.Duration()))
 		} else {
-			log.Warningf("not caching %s cluster costs: no data or less than %f minutes data ", duration, clusterCostsCacheMinutes)
+			log.Warningf("not caching %s cluster costs: no data or less than %f minutes data ", fmtDuration, clusterCostsCacheMinutes)
 		}
 		return aggErr, err
 	}
@@ -1811,18 +1813,16 @@ func (a *Accesses) warmAggregateCostModelCache() {
 	go func(sem *util.Semaphore) {
 		defer errors.HandlePanic()
 
-		duration := "1d"
-		offset := "1m"
-		durHrs := "24h"
-		dur := 24 * time.Hour
+		offset := time.Minute
+		duration := 24 * time.Hour
 
 		for {
 			sem.Acquire()
-			warmFunc(duration, durHrs, offset, true)
+			warmFunc(duration, offset, true)
 			sem.Return()
 
-			log.Infof("aggregation: warm cache: %s", duration)
-			time.Sleep(a.GetCacheRefresh(dur))
+			log.Infof("aggregation: warm cache: %s", timeutil.DurationString(duration))
+			time.Sleep(a.GetCacheRefresh(duration))
 		}
 	}(sem)
 
@@ -1831,18 +1831,16 @@ func (a *Accesses) warmAggregateCostModelCache() {
 		go func(sem *util.Semaphore) {
 			defer errors.HandlePanic()
 
-			duration := "2d"
-			offset := "1m"
-			durHrs := "48h"
-			dur := 2 * 24 * time.Hour
+			offset := time.Minute
+			duration := 2 * 24 * time.Hour
 
 			for {
 				sem.Acquire()
-				warmFunc(duration, durHrs, offset, false)
+				warmFunc(duration, offset, false)
 				sem.Return()
 
-				log.Infof("aggregation: warm cache: %s", duration)
-				time.Sleep(a.GetCacheRefresh(dur))
+				log.Infof("aggregation: warm cache: %s", timeutil.DurationString(duration))
+				time.Sleep(a.GetCacheRefresh(duration))
 			}
 		}(sem)
 
@@ -1850,19 +1848,17 @@ func (a *Accesses) warmAggregateCostModelCache() {
 		go func(sem *util.Semaphore) {
 			defer errors.HandlePanic()
 
-			duration := "7d"
-			offset := "1m"
-			durHrs := "168h"
-			dur := 7 * 24 * time.Hour
+			offset := time.Minute
+			duration := 7 * 24 * time.Hour
 
 			for {
 				sem.Acquire()
-				aggErr, err := warmFunc(duration, durHrs, offset, false)
+				aggErr, err := warmFunc(duration, offset, false)
 				sem.Return()
 
-				log.Infof("aggregation: warm cache: %s", duration)
+				log.Infof("aggregation: warm cache: %s", timeutil.DurationString(duration))
 				if aggErr == nil && err == nil {
-					time.Sleep(a.GetCacheRefresh(dur))
+					time.Sleep(a.GetCacheRefresh(duration))
 				} else {
 					time.Sleep(5 * time.Minute)
 				}
@@ -1874,16 +1870,14 @@ func (a *Accesses) warmAggregateCostModelCache() {
 			defer errors.HandlePanic()
 
 			for {
-				duration := "30d"
-				offset := "1m"
-				durHrs := "720h"
-				dur := 30 * 24 * time.Hour
+				offset := time.Minute
+				duration := 30 * 24 * time.Hour
 
 				sem.Acquire()
-				aggErr, err := warmFunc(duration, durHrs, offset, false)
+				aggErr, err := warmFunc(duration, offset, false)
 				sem.Return()
 				if aggErr == nil && err == nil {
-					time.Sleep(a.GetCacheRefresh(dur))
+					time.Sleep(a.GetCacheRefresh(duration))
 				} else {
 					time.Sleep(5 * time.Minute)
 				}

pkg/costmodel/cluster.go

@@ -70,16 +70,12 @@ type ClusterCostsBreakdown struct {
 
 // NewClusterCostsFromCumulative takes cumulative cost data over a given time range, computes
 // the associated monthly rate data, and returns the Costs.
-func NewClusterCostsFromCumulative(cpu, gpu, ram, storage float64, window, offset string, dataHours float64) (*ClusterCosts, error) {
-	offset = timeutil.CleanDurationString(offset)
-	start, end, err := timeutil.ParseTimeRange(window, offset)
-	if err != nil {
-		return nil, err
-	}
+func NewClusterCostsFromCumulative(cpu, gpu, ram, storage float64, window, offset time.Duration, dataHours float64) (*ClusterCosts, error) {
+	start, end := timeutil.ParseTimeRange(window, offset)
 
 	// If the number of hours is not given (i.e. is zero) compute one from the window and offset
 	if dataHours == 0 {
-		dataHours = end.Sub(*start).Hours()
+		dataHours = end.Sub(start).Hours()
 	}
 
 	// Do not allow zero-length windows to prevent divide-by-zero issues
@@ -88,8 +84,8 @@ func NewClusterCostsFromCumulative(cpu, gpu, ram, storage float64, window, offse
 	}
 
 	cc := &ClusterCosts{
-		Start:             start,
-		End:               end,
+		Start:             &start,
+		End:               &end,
 		CPUCumulative:     cpu,
 		GPUCumulative:     gpu,
 		RAMCumulative:     ram,
@@ -703,14 +699,11 @@ func ClusterLoadBalancers(cp cloud.Provider, client prometheus.Client, duration,
 }
 
 // ComputeClusterCosts gives the cumulative and monthly-rate cluster costs over a window of time for all clusters.
-func (a *Accesses) ComputeClusterCosts(client prometheus.Client, provider cloud.Provider, window, offset string, withBreakdown bool) (map[string]*ClusterCosts, error) {
+func (a *Accesses) ComputeClusterCosts(client prometheus.Client, provider cloud.Provider, window, offset time.Duration, withBreakdown bool) (map[string]*ClusterCosts, error) {
 	// Compute number of minutes in the full interval, for use interpolating missed scrapes or scaling missing data
-	offset = timeutil.CleanDurationString(offset)
-	start, end, err := timeutil.ParseTimeRange(window, offset)
-	if err != nil {
-		return nil, err
-	}
-	mins := end.Sub(*start).Minutes()
+	start, end := timeutil.ParseTimeRange(window, offset)
+
+	mins := end.Sub(start).Minutes()
 
 	// minsPerResolution determines accuracy and resource use for the following
 	// queries. Smaller values (higher resolution) result in better accuracy,
@@ -779,10 +772,7 @@ func (a *Accesses) ComputeClusterCosts(client prometheus.Client, provider cloud.
 		queryTotalLocalStorage = fmt.Sprintf(" + %s", queryTotalLocalStorage)
 	}
 
-	fmtOffset := ""
-	if offset != "" {
-		fmtOffset = fmt.Sprintf("offset %s", offset)
-	}
+	fmtOffset := timeutil.DurationToPromString(offset)
 
 	queryDataCount := fmt.Sprintf(fmtQueryDataCount, env.GetPromClusterLabel(), window, minsPerResolution, fmtOffset, minsPerResolution)
 	queryTotalGPU := fmt.Sprintf(fmtQueryTotalGPU, window, minsPerResolution, fmtOffset, hourlyToCumulative, env.GetPromClusterLabel())
@@ -1056,8 +1046,8 @@ func resultToTotals(qrs []*prom.QueryResult) ([][]string, error) {
 }
 
 // ClusterCostsOverTime gives the full cluster costs over time
-func ClusterCostsOverTime(cli prometheus.Client, provider cloud.Provider, startString, endString, windowString, offset string) (*Totals, error) {
-	localStorageQuery := provider.GetLocalStorageQuery(windowString, offset, true, false)
+func ClusterCostsOverTime(cli prometheus.Client, provider cloud.Provider, startString, endString string, window, offset time.Duration) (*Totals, error) {
+	localStorageQuery := provider.GetLocalStorageQuery(window, offset, true, false)
 	if localStorageQuery != "" {
 		localStorageQuery = fmt.Sprintf("+ %s", localStorageQuery)
 	}
@@ -1066,28 +1056,27 @@ func ClusterCostsOverTime(cli prometheus.Client, provider cloud.Provider, startS
 
 	start, err := time.Parse(layout, startString)
 	if err != nil {
-		klog.V(1).Infof("Error parsing time " + startString + ". Error: " + err.Error())
+		klog.V(1).Infof("Error parsing time %s. Error: %s", startString, err.Error())
 		return nil, err
 	}
 	end, err := time.Parse(layout, endString)
 	if err != nil {
-		klog.V(1).Infof("Error parsing time " + endString + ". Error: " + err.Error())
+		klog.V(1).Infof("Error parsing time %s. Error: %s", endString, err.Error())
 		return nil, err
 	}
-	window, err := time.ParseDuration(windowString)
-	if err != nil {
-		klog.V(1).Infof("Error parsing time " + windowString + ". Error: " + err.Error())
+	fmtWindow := timeutil.DurationString(window)
+
+	if fmtWindow == "" {
+		err := fmt.Errorf("window value invalid or missing")
+		klog.V(1).Infof("Error parsing time %v. Error: %s", window, err.Error())
 		return nil, err
 	}
 
-	// turn offsets of the format "[0-9+]h" into the format "offset [0-9+]h" for use in query templatess
-	if offset != "" {
-		offset = fmt.Sprintf("offset %s", offset)
-	}
+	fmtOffset := timeutil.DurationToPromString(offset)
 
-	qCores := fmt.Sprintf(queryClusterCores, windowString, offset, env.GetPromClusterLabel(), windowString, offset, env.GetPromClusterLabel(), windowString, offset, env.GetPromClusterLabel(), env.GetPromClusterLabel())
-	qRAM := fmt.Sprintf(queryClusterRAM, windowString, offset, env.GetPromClusterLabel(), windowString, offset, env.GetPromClusterLabel(), env.GetPromClusterLabel())
-	qStorage := fmt.Sprintf(queryStorage, windowString, offset, env.GetPromClusterLabel(), windowString, offset, env.GetPromClusterLabel(), env.GetPromClusterLabel(), localStorageQuery)
+	qCores := fmt.Sprintf(queryClusterCores, fmtWindow, fmtOffset, env.GetPromClusterLabel(), fmtWindow, fmtOffset, env.GetPromClusterLabel(), fmtWindow, fmtOffset, env.GetPromClusterLabel(), env.GetPromClusterLabel())
+	qRAM := fmt.Sprintf(queryClusterRAM, fmtWindow, fmtOffset, env.GetPromClusterLabel(), fmtWindow, fmtOffset, env.GetPromClusterLabel(), env.GetPromClusterLabel())
+	qStorage := fmt.Sprintf(queryStorage, fmtWindow, fmtOffset, env.GetPromClusterLabel(), fmtWindow, fmtOffset, env.GetPromClusterLabel(), env.GetPromClusterLabel(), localStorageQuery)
 	qTotal := fmt.Sprintf(queryTotal, env.GetPromClusterLabel(), env.GetPromClusterLabel(), env.GetPromClusterLabel(), env.GetPromClusterLabel(), localStorageQuery)
 
 	ctx := prom.NewContext(cli)

pkg/costmodel/router.go

@@ -124,16 +124,18 @@ func (a *Accesses) GetCacheRefresh(dur time.Duration) time.Duration {
 func (a *Accesses) ClusterCostsFromCacheHandler(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
 	w.Header().Set("Content-Type", "application/json")
 
+	duration := 24 * time.Hour
+	offset := time.Minute
 	durationHrs := "24h"
-	offset := "1m"
+	fmtOffset := "1m"
 	pClient := a.GetPrometheusClient(true)
 
-	key := fmt.Sprintf("%s:%s", durationHrs, offset)
+	key := fmt.Sprintf("%s:%s", durationHrs, fmtOffset)
 	if data, valid := a.ClusterCostsCache.Get(key); valid {
 		clusterCosts := data.(map[string]*ClusterCosts)
 		w.Write(WrapDataWithMessage(clusterCosts, nil, "clusterCosts cache hit"))
 	} else {
-		data, err := a.ComputeClusterCosts(pClient, a.CloudProvider, durationHrs, offset, true)
+		data, err := a.ComputeClusterCosts(pClient, a.CloudProvider, duration, offset, true)
 		w.Write(WrapDataWithMessage(data, err, fmt.Sprintf("clusterCosts cache miss: %s", key)))
 	}
 }
@@ -244,38 +246,6 @@ func ParsePercentString(percentStr string) (float64, error) {
 	return discount, nil
 }
 
-// ParseTimeRange returns a start and end time, respectively, which are converted from
-// a duration and offset, defined as strings with Prometheus-style syntax.
-func ParseTimeRange(duration, offset string) (*time.Time, *time.Time, error) {
-	// endTime defaults to the current time, unless an offset is explicity declared,
-	// in which case it shifts endTime back by given duration
-	endTime := time.Now()
-	if offset != "" {
-		o, err := timeutil.ParseDuration(offset)
-		if err != nil {
-			return nil, nil, fmt.Errorf("error parsing offset (%s): %s", offset, err)
-		}
-		endTime = endTime.Add(-1 * *o)
-	}
-
-	// if duration is defined in terms of days, convert to hours
-	// e.g. convert "2d" to "48h"
-	durationNorm, err := normalizeTimeParam(duration)
-	if err != nil {
-		return nil, nil, fmt.Errorf("error parsing duration (%s): %s", duration, err)
-	}
-
-	// convert time duration into start and end times, formatted
-	// as ISO datetime strings
-	dur, err := time.ParseDuration(durationNorm)
-	if err != nil {
-		return nil, nil, fmt.Errorf("errorf parsing duration (%s): %s", durationNorm, err)
-	}
-	startTime := endTime.Add(-1 * dur)
-
-	return &startTime, &endTime, nil
-}
-
 func WrapData(data interface{}, err error) []byte {
 	var resp []byte
 
@@ -411,6 +381,27 @@ func (a *Accesses) ClusterCosts(w http.ResponseWriter, r *http.Request, ps httpr
 	window := r.URL.Query().Get("window")
 	offset := r.URL.Query().Get("offset")
 
+	if window == "" {
+		w.Write(WrapData(nil, fmt.Errorf("missing window arguement")))
+		return
+	}
+	windowDur, err := timeutil.ParseDuration(window)
+	if err != nil {
+		w.Write(WrapData(nil, fmt.Errorf("error parsing window (%s): %s", window, err)))
+		return
+	}
+
+	// offset is not a required parameter
+	var offsetDur time.Duration
+	if offset != "" {
+		offsetDur, err = timeutil.ParseDuration(offset)
+		if err != nil {
+			w.Write(WrapData(nil, fmt.Errorf("error parsing offset (%s): %s", offset, err)))
+			return
+		}
+	}
+
+
 	useThanos, _ := strconv.ParseBool(r.URL.Query().Get("multi"))
 
 	if useThanos && !thanos.IsEnabled() {
@@ -421,12 +412,13 @@ func (a *Accesses) ClusterCosts(w http.ResponseWriter, r *http.Request, ps httpr
 	var client prometheusClient.Client
 	if useThanos {
 		client = a.ThanosClient
-		offset = thanos.Offset()
+		offsetDur = thanos.OffsetDuration()
+
 	} else {
 		client = a.PrometheusClient
 	}
 
-	data, err := a.ComputeClusterCosts(client, a.CloudProvider, window, offset, true)
+	data, err := a.ComputeClusterCosts(client, a.CloudProvider, windowDur, offsetDur, true)
 	w.Write(WrapData(data, err))
 }
 
@@ -439,7 +431,27 @@ func (a *Accesses) ClusterCostsOverTime(w http.ResponseWriter, r *http.Request,
 	window := r.URL.Query().Get("window")
 	offset := r.URL.Query().Get("offset")
 
-	data, err := ClusterCostsOverTime(a.PrometheusClient, a.CloudProvider, start, end, window, offset)
+	if window == "" {
+		w.Write(WrapData(nil, fmt.Errorf("missing window arguement")))
+		return
+	}
+	windowDur, err := timeutil.ParseDuration(window)
+	if err != nil {
+		w.Write(WrapData(nil, fmt.Errorf("error parsing window (%s): %s", window, err)))
+		return
+	}
+
+	// offset is not a required parameter
+	var offsetDur time.Duration
+	if offset != "" {
+		offsetDur, err = timeutil.ParseDuration(offset)
+		if err != nil {
+			w.Write(WrapData(nil, fmt.Errorf("error parsing offset (%s): %s", offset, err)))
+			return
+		}
+	}
+
+	data, err := ClusterCostsOverTime(a.PrometheusClient, a.CloudProvider, start, end, windowDur, offsetDur)
 	w.Write(WrapData(data, err))
 }
 

pkg/util/mapper/mapper.go

@@ -402,7 +402,7 @@ func (rom *readOnlyMapper) GetDuration(key string, defaultValue time.Duration) t
 		return defaultValue
 	}
 
-	return *d
+	return d
 }
 
 // GetList returns a string list which contains the value set by key split using the

pkg/util/timeutil/timeutil.go

@@ -51,7 +51,7 @@ func DurationString(duration time.Duration) string {
 			// convert to mins
 			durStr = fmt.Sprintf("%dm", durSecs/SecsPerMin)
 		} else if durSecs > 0 {
-			// default to mins, as long as duration is positive
+			// default to secs, as long as duration is positive
 			durStr = fmt.Sprintf("%ds", durSecs)
 		}
 	}
@@ -59,6 +59,15 @@ func DurationString(duration time.Duration) string {
 	return durStr
 }
 
+// DurationToPromString returns a Prometheus formatted string with leading offset or empty string if given a negative duration
+func DurationToPromString(duration time.Duration) string {
+	dirStr := DurationString(duration)
+	if dirStr != "" {
+		dirStr = fmt.Sprintf("offset %s", dirStr)
+	}
+	return dirStr
+}
+
 // DurationOffsetStrings converts a (duration, offset) pair to Prometheus-
 // compatible strings in terms of days, hours, minutes, or seconds.
 func DurationOffsetStrings(duration, offset time.Duration) (string, string) {
@@ -154,11 +163,11 @@ func (jt *JobTicker) TickIn(d time.Duration) {
 
 
 // ParseDuration converts a Prometheus-style duration string into a Duration
-func ParseDuration(duration string) (*time.Duration, error) {
+func ParseDuration(duration string) (time.Duration, error) {
 	// Trim prefix of Prometheus format duration
 	duration = CleanDurationString(duration)
 	if len(duration) < 2 {
-		return nil, fmt.Errorf("error parsing duration: %s did not match expected format [0-9+](s|m|d|h)", duration)
+		return 0, fmt.Errorf("error parsing duration: %s did not match expected format [0-9+](s|m|d|h)", duration)
 	}
 	unitStr := duration[len(duration)-1:]
 	var unit time.Duration
@@ -172,17 +181,16 @@ func ParseDuration(duration string) (*time.Duration, error) {
 	case "d":
 		unit = 24.0 * time.Hour
 	default:
-		return nil, fmt.Errorf("error parsing duration: %s did not match expected format [0-9+](s|m|d|h)", duration)
+		return 0, fmt.Errorf("error parsing duration: %s did not match expected format [0-9+](s|m|d|h)", duration)
 	}
 
 	amountStr := duration[:len(duration)-1]
 	amount, err := strconv.ParseInt(amountStr, 10, 64)
 	if err != nil {
-		return nil, fmt.Errorf("error parsing duration: %s did not match expected format [0-9+](s|m|d|h)", duration)
+		return 0, fmt.Errorf("error parsing duration: %s did not match expected format [0-9+](s|m|d|h)", duration)
 	}
 
-	dur := time.Duration(amount) * unit
-	return &dur, nil
+	return time.Duration(amount) * unit, nil
 }
 
 // CleanDurationString removes prometheus formatted prefix "offset " allong with leading a trailing whitespace
@@ -195,37 +203,21 @@ func CleanDurationString(duration string) string {
 
 // ParseTimeRange returns a start and end time, respectively, which are converted from
 // a duration and offset, defined as strings with Prometheus-style syntax.
-func ParseTimeRange(duration, offset string) (*time.Time, *time.Time, error) {
+func ParseTimeRange(duration, offset time.Duration) (time.Time, time.Time) {
 	// endTime defaults to the current time, unless an offset is explicity declared,
 	// in which case it shifts endTime back by given duration
 	endTime := time.Now()
-	if offset != "" {
-		o, err := ParseDuration(offset)
-		if err != nil {
-			return nil, nil, fmt.Errorf("error parsing offset (%s): %s", offset, err)
-		}
-		endTime = endTime.Add(-1 * *o)
+	if offset > 0 {
+		endTime = endTime.Add(-1 * offset)
 	}
 
-	// if duration is defined in terms of days, convert to hours
-	// e.g. convert "2d" to "48h"
-	durationNorm, err := normalizeTimeParam(duration)
-	if err != nil {
-		return nil, nil, fmt.Errorf("error parsing duration (%s): %s", duration, err)
-	}
-
-	// convert time duration into start and end times, formatted
-	// as ISO datetime strings
-	dur, err := time.ParseDuration(durationNorm)
-	if err != nil {
-		return nil, nil, fmt.Errorf("errorf parsing duration (%s): %s", durationNorm, err)
-	}
-	startTime := endTime.Add(-1 * dur)
+	startTime := endTime.Add(-1 * duration)
 
-	return &startTime, &endTime, nil
+	return startTime, endTime
 }
 
-func normalizeTimeParam(param string) (string, error) {
+// DayDurationToHourDuration converts string from format [0-9+]d to [0-9+]h
+func DayDurationToHourDuration(param string) (string, error) {
 	// convert days to hours
 	if param[len(param)-1:] == "d" {
 		count := param[:len(param)-1]

pkg/util/timeutil/timeutil_test.go

@@ -60,59 +60,52 @@ func TestParseDuration(t *testing.T) {
 	testCases := map[string]struct {
 		input string
 		expected time.Duration
-		errorExpected bool
 	} {
 		"expected" : {
 			input: "3h",
 			expected: time.Hour * 3,
-			errorExpected: false,
 		},
 		"white space" : {
 			input: " 4s ",
 			expected: time.Second * 4,
-			errorExpected: false,
 		},
 		"prom prefix" : {
 			input: "offset 3m",
 			expected: time.Minute * 3,
-			errorExpected: false,
 		},
 		"prom prefix white space" : {
 			input: " offset 3d ",
 			expected: 24.0 * time.Hour * 3,
-			errorExpected: false,
+		},
+		"zero" : {
+			input: "0h",
+			expected: time.Duration(0),
 		},
 		"empty" : {
 			input: "",
-			expected: time.Second,
-			errorExpected: true,
+			expected: time.Duration(0),
 		},
 		"bad string" : {
 			input: "oqwd3dk5hk",
-			expected: time.Second,
-			errorExpected: true,
+			expected: time.Duration(0),
 		},
 		"digit" : {
 			input: "3",
-			expected: time.Second,
-			errorExpected: true,
+			expected: time.Duration(0),
 		},
 		"unit" : {
 			input: "h",
-			expected: time.Second,
-			errorExpected: true,
+			expected: time.Duration(0),
 		},
 	}
 	for name, test := range testCases {
 		t.Run(name, func(t *testing.T) {
-			dur, err := ParseDuration(test.input)
-			if err != nil && test.errorExpected{
-				return
-			}
-			if *dur != test.expected {
+			dur, _ := ParseDuration(test.input)
+			if dur != test.expected {
 				t.Errorf("Expected duration %v did not match result %v", test.expected, dur)
 			}
 		})
 	}
-
 }
+
+