浏览代码

Merge branch 'sean/add-annotations' into sean/agg-on-annotations

Sean Holcomb 5 年之前
父节点
当前提交
97f205c011

+ 1 - 1
pkg/cloud/awsprovider.go

@@ -265,7 +265,7 @@ var locationToRegion = map[string]string{
 	"EU (Stockholm)":             "eu-north-1",
 	"South America (Sao Paulo)":  "sa-east-1",
 	"AWS GovCloud (US-East)":     "us-gov-east-1",
-	"AWS GovCloud (US)":          "us-gov-west-1",
+	"AWS GovCloud (US-West)":     "us-gov-west-1",
 }
 
 var regionToBillingRegionCode = map[string]string{

+ 27 - 11
pkg/costmodel/aggregation.go

@@ -1462,22 +1462,38 @@ func (a *Accesses) ComputeAggregateCostModel(promClient prometheusClient.Client,
 
 	idleCoefficients := make(map[string]float64)
 	if allocateIdle {
-		duration, offset := window.DurationOffsetStrings()
-
-		idleDurationCalcHours := window.Hours()
-		if window.Hours() < 1 {
-			idleDurationCalcHours = 1
+		dur, off, err := window.DurationOffset()
+		if err != nil {
+			log.Errorf("ComputeAggregateCostModel: error computing idle coefficient: illegal window: %s (%s)", window, err)
+			return nil, "", err
 		}
-		duration = fmt.Sprintf("%dh", int(idleDurationCalcHours))
 
-		if a.ThanosClient != nil {
-			offset = thanos.Offset()
-			log.Infof("ComputeAggregateCostModel: setting offset to %s", offset)
+		if a.ThanosClient != nil && off < thanos.OffsetDuration() {
+			// Determine difference between the Thanos offset and the requested
+			// offset; e.g. off=1h, thanosOffsetDuration=3h => diff=2h
+			diff := thanos.OffsetDuration() - off
+
+			// Reduce duration by difference and increase offset by difference
+			// e.g. 24h offset 0h => 21h offset 3h
+			dur = dur - diff
+			off = thanos.OffsetDuration()
+
+			log.Infof("ComputeAggregateCostModel: setting duration, offset to %s, %s due to Thanos", dur, off)
+
+			// Idle computation cannot be fulfilled for some windows, specifically
+			// those with sum(duration, offset) < Thanos offset, because there is
+			// no data within that window.
+			if dur <= 0 {
+				return nil, "", fmt.Errorf("requested idle coefficients from Thanos for illegal duration, offset: %s, %s (original window %s)", dur, off, window)
+			}
 		}
 
-		idleCoefficients, err = a.ComputeIdleCoefficient(costData, promClient, a.CloudProvider, discount, customDiscount, duration, offset)
+		// Convert to Prometheus-compatible strings
+		durStr, offStr := util.DurationOffsetStrings(dur, off)
+
+		idleCoefficients, err = a.ComputeIdleCoefficient(costData, promClient, a.CloudProvider, discount, customDiscount, durStr, offStr)
 		if err != nil {
-			log.Errorf("ComputeAggregateCostModel: error computing idle coefficient: duration=%s, offset=%s, err=%s", duration, offset, err)
+			log.Errorf("ComputeAggregateCostModel: error computing idle coefficient: duration=%s, offset=%s, err=%s", durStr, offStr, err)
 			return nil, "", err
 		}
 	}

+ 12 - 12
pkg/costmodel/containerkeys.go

@@ -5,7 +5,7 @@ import (
 	"strings"
 
 	"github.com/kubecost/cost-model/pkg/log"
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 )
 
 var (
@@ -13,22 +13,22 @@ var (
 	NewKeyTupleErr = errors.New("NewKeyTuple() Provided key not containing exactly 3 components.")
 
 	// Static Errors for ContainerMetric creation
-	InvalidKeyErr error = errors.New("Not a valid key")
-	NoContainerErr error = errors.New("Prometheus vector does not have container name")
+	InvalidKeyErr      error = errors.New("Not a valid key")
+	NoContainerErr     error = errors.New("Prometheus vector does not have container name")
 	NoContainerNameErr error = errors.New("Prometheus vector does not have string container name")
-	NoPodErr error = errors.New("Prometheus vector does not have pod name")
-	NoPodNameErr error = errors.New("Prometheus vector does not have string pod name")
-	NoNamespaceErr error = errors.New("Prometheus vector does not have namespace")
+	NoPodErr           error = errors.New("Prometheus vector does not have pod name")
+	NoPodNameErr       error = errors.New("Prometheus vector does not have string pod name")
+	NoNamespaceErr     error = errors.New("Prometheus vector does not have namespace")
 	NoNamespaceNameErr error = errors.New("Prometheus vector does not have string namespace")
-	NoNodeNameErr error = errors.New("Prometheus vector does not have string node")
-	NoClusterIDErr error = errors.New("Prometheus vector does not have string cluster_id")
+	NoNodeNameErr      error = errors.New("Prometheus vector does not have string node")
+	NoClusterIDErr     error = errors.New("Prometheus vector does not have string cluster_id")
 )
 
 //--------------------------------------------------------------------------
 //  KeyTuple
 //--------------------------------------------------------------------------
 
-// KeyTuple contains is a utility which parses Namespace, Key, and ClusterID from a 
+// KeyTuple contains is a utility which parses Namespace, Key, and ClusterID from a
 // comma delimitted string.
 type KeyTuple struct {
 	key    string
@@ -103,8 +103,8 @@ func containerMetricKey(ns, podName, containerName, nodeName, clusterID string)
 	return ns + "," + podName + "," + containerName + "," + nodeName + "," + clusterID
 }
 
-// NewContainerMetricFromKey creates a new ContainerMetric instance using a provided comma delimitted 
-// string key. 
+// NewContainerMetricFromKey creates a new ContainerMetric instance using a provided comma delimitted
+// string key.
 func NewContainerMetricFromKey(key string) (*ContainerMetric, error) {
 	s := strings.Split(key, ",")
 	if len(s) == 5 {
@@ -132,7 +132,7 @@ func NewContainerMetricFromValues(ns, podName, containerName, nodeName, clusterI
 	}
 }
 
-// NewContainerMetricsFromPod creates a slice of ContainerMetric instances for each container in the 
+// NewContainerMetricsFromPod creates a slice of ContainerMetric instances for each container in the
 // provided Pod.
 func NewContainerMetricsFromPod(pod *v1.Pod, clusterID string) ([]*ContainerMetric, error) {
 	podName := pod.GetObjectMeta().GetName()

+ 3 - 3
pkg/costmodel/costmodel.go

@@ -178,10 +178,10 @@ const (
 		)
 	) by (namespace,container_name,pod_name,node,cluster_id)
 	* on (pod_name, namespace, cluster_id) group_left(container) label_replace(avg(avg_over_time(kube_pod_status_phase{phase="Running"}[%s] %s)) by (pod,namespace,cluster_id), "pod_name","$1","pod","(.+)")`
-	queryPVRequestsStr = `avg(avg(kube_persistentvolumeclaim_info{volumename != ""}) by (persistentvolumeclaim, storageclass, namespace, volumename, cluster_id)
+	queryPVRequestsStr = `avg(avg(kube_persistentvolumeclaim_info{volumename != ""}) by (persistentvolumeclaim, storageclass, namespace, volumename, cluster_id, kubernetes_node)
 	*
-	on (persistentvolumeclaim, namespace, cluster_id) group_right(storageclass, volumename)
-	sum(kube_persistentvolumeclaim_resource_requests_storage_bytes{volumename != ""}) by (persistentvolumeclaim, namespace, cluster_id, kubernetes_name)) by (persistentvolumeclaim, storageclass, namespace, volumename, cluster_id)`
+	on (persistentvolumeclaim, namespace, cluster_id, kubernetes_node) group_right(storageclass, volumename)
+	sum(kube_persistentvolumeclaim_resource_requests_storage_bytes{}) by (persistentvolumeclaim, namespace, cluster_id, kubernetes_node, kubernetes_name)) by (persistentvolumeclaim, storageclass, namespace, cluster_id, volumename, kubernetes_node)`
 	// queryRAMAllocationByteHours yields the total byte-hour RAM allocation over the given
 	// window, aggregated by container.
 	//  [line 3]  sum_over_time(each byte) = [byte*scrape] by metric

+ 6 - 1
pkg/costmodel/metrics.go

@@ -13,6 +13,7 @@ import (
 	"github.com/kubecost/cost-model/pkg/errors"
 	"github.com/kubecost/cost-model/pkg/log"
 	"github.com/kubecost/cost-model/pkg/prom"
+	"github.com/kubecost/cost-model/pkg/util"
 
 	promclient "github.com/prometheus/client_golang/api"
 	"github.com/prometheus/client_golang/prometheus"
@@ -790,7 +791,11 @@ func (cmme *CostModelMetricsEmitter) Start() bool {
 		var defaultRegion string = ""
 		nodeList := cmme.KubeClusterCache.GetAllNodes()
 		if len(nodeList) > 0 {
-			defaultRegion = nodeList[0].Labels[v1.LabelZoneRegion]
+			var ok bool
+			defaultRegion, ok = util.GetRegion(nodeList[0].Labels)
+			if !ok {
+				log.DedupedWarningf(5, "Failed to locate default region")
+			}
 		}
 
 		for {

+ 1 - 1
pkg/env/costmodelenv.go

@@ -64,7 +64,7 @@ const (
 // GetAWSAccessKeyID returns the environment variable value for AWSAccessKeyIDEnvVar which represents
 // the AWS access key for authentication
 func GetAppVersion() string {
-	return Get(AppVersionEnvVar, "1.71.0")
+	return Get(AppVersionEnvVar, "1.72.0")
 }
 
 // IsEmitNamespaceAnnotationsMetric returns true if cost-model is configured to emit the kube_namespace_annotations metric

+ 36 - 0
pkg/kubecost/allocation.go

@@ -959,6 +959,42 @@ func (alloc *Allocation) generateKey(properties Properties) (string, error) {
 		}
 	}
 
+	if properties.HasAnnotations() {
+		annotations, err := alloc.Properties.GetAnnotations() // annotations that the individual allocation possesses
+		if err != nil {
+			// Indicate that allocation has no annotations
+			names = append(names, UnallocatedSuffix)
+		} else {
+			annotationNames := []string{}
+
+			aggAnnotations, err := properties.GetAnnotations() // potential annotations to aggregate on supplied by the API caller
+			if err != nil {
+				// We've already checked HasAnnotation, so this should never occur
+				return "", err
+			}
+			// calvin - support multi-annotation aggregation
+			for annotationName := range aggAnnotations {
+				if val, ok := annotations[annotationName]; ok {
+					annotationNames = append(annotationNames, fmt.Sprintf("%s=%s", annotationName, val))
+				} else if indexOf(UnallocatedSuffix, annotationNames) == -1 { // if UnallocatedSuffix not already in names
+					annotationNames = append(annotationNames, UnallocatedSuffix)
+				}
+			}
+			// resolve arbitrary ordering. e.g., app=app0/env=env0 is the same agg as env=env0/app=app0
+			if len(annotationNames) > 1 {
+				sort.Strings(annotationNames)
+			}
+			unallocatedSuffixIndex := indexOf(UnallocatedSuffix, annotationNames)
+			// suffix should be at index 0 if it exists b/c of underscores
+			if unallocatedSuffixIndex != -1 {
+				annotationNames = append(annotationNames[:unallocatedSuffixIndex], annotationNames[unallocatedSuffixIndex+1:]...)
+				annotationNames = append(annotationNames, UnallocatedSuffix) // append to end
+			}
+
+			names = append(names, annotationNames...)
+		}
+	}
+
 	if properties.HasLabel() {
 		labels, err := alloc.Properties.GetLabels() // labels that the individual allocation possesses
 		if err != nil {

+ 39 - 4
pkg/kubecost/allocation_test.go

@@ -208,7 +208,7 @@ func generateAllocationSet(start time.Time) *AllocationSet {
 	// Idle allocations
 	a1i := NewUnitAllocation(fmt.Sprintf("cluster1/%s", IdleSuffix), start, day, &Properties{
 		ClusterProp: "cluster1",
-		NodeProp: "node1",
+		NodeProp:    "node1",
 	})
 	a1i.CPUCost = 5.0
 	a1i.RAMCost = 15.0
@@ -347,6 +347,11 @@ func generateAllocationSet(start time.Time) *AllocationSet {
 	a22mno4.Properties.SetLabels(map[string]string{"app": "app2"})
 	a22mno5.Properties.SetLabels(map[string]string{"app": "app2"})
 
+	//Annotations
+	a23stu7.Properties.SetAnnotations(map[string]string{"team": "team1"})
+	a23vwx8.Properties.SetAnnotations(map[string]string{"team": "team2"})
+	a23vwx9.Properties.SetAnnotations(map[string]string{"team": "team1"})
+
 	// Services
 
 	a12jkl6.Properties.SetServices([]string{"service1"})
@@ -445,10 +450,10 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 	//         container6: {service1}              5.00   1.00   1.00   1.00   1.00   1.00
 	//     namespace3:
 	//       pod-stu: (deployment3)
-	//         container7:                         5.00   1.00   1.00   1.00   1.00   1.00
+	//         container7: an[team=team1]          5.00   1.00   1.00   1.00   1.00   1.00
 	//       pod-vwx: (statefulset1)
-	//         container8:                         5.00   1.00   1.00   1.00   1.00   1.00
-	//         container9:                         5.00   1.00   1.00   1.00   1.00   1.00
+	//         container8: an[team=team2]          5.00   1.00   1.00   1.00   1.00   1.00
+	//         container9: an[team=team1]          5.00   1.00   1.00   1.00   1.00   1.00
 	// +----------------------------------------+------+------+------+------+------+------+
 	//   cluster2 subtotal                        40.00  11.00  11.00   6.00   6.00   6.00
 	// +----------------------------------------+------+------+------+------+------+------+
@@ -669,6 +674,18 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 	})
 	assertAllocationWindow(t, as, "1i", startYesterday, endYesterday, 1440.0)
 
+	// 1j AggregationProperties=(Annotation:team)
+	as = generateAllocationSet(start)
+	err = as.AggregateBy(Properties{AnnotationProp: map[string]string{"team": ""}}, nil)
+	assertAllocationSetTotals(t, as, "1j", err, 2+numIdle+numUnallocated, activeTotalCost+idleTotalCost)
+	assertAllocationTotals(t, as, "1j", map[string]float64{
+		"team=team1":      10.00,
+		"team=team2":      5.00,
+		IdleSuffix:        30.00,
+		UnallocatedSuffix: 55.00,
+	})
+	assertAllocationWindow(t, as, "1i", startYesterday, endYesterday, 1440.0)
+
 	// 2  Multi-aggregation
 
 	// 2a AggregationProperties=(Cluster, Namespace)
@@ -701,6 +718,24 @@ func TestAllocationSet_AggregateBy(t *testing.T) {
 		"cluster2/" + UnallocatedSuffix:          20.00,
 	})
 
+	// 2f AggregationProperties=(annotation:team, pod)
+	as = generateAllocationSet(start)
+	err = as.AggregateBy(Properties{AnnotationProp: map[string]string{"team": ""}, PodProp: ""}, nil)
+	assertAllocationSetTotals(t, as, "2e", err, 11, activeTotalCost+idleTotalCost)
+	assertAllocationTotals(t, as, "2e", map[string]float64{
+		"pod-jkl/" + UnallocatedSuffix: 5.00,
+		"pod-stu/team=team1":           5.00,
+		"pod-abc/" + UnallocatedSuffix: 5.00,
+		"pod-pqr/" + UnallocatedSuffix: 5.00,
+		"pod-def/" + UnallocatedSuffix: 5.00,
+		"pod-vwx/team=team1":           5.00,
+		"pod-vwx/team=team2":           5.00,
+		"pod1/" + UnallocatedSuffix:    15.00,
+		"pod-mno/" + UnallocatedSuffix: 10.00,
+		"pod-ghi/" + UnallocatedSuffix: 10.00,
+		IdleSuffix:                     30.00,
+	})
+
 	// // TODO niko/etl
 
 	// // 3  Share idle

+ 4 - 6
pkg/kubecost/asset.go

@@ -114,7 +114,7 @@ type AssetLabels map[string]string
 
 // Clone returns a cloned map of labels
 func (al AssetLabels) Clone() AssetLabels {
-	clone := AssetLabels{}
+	clone := make(AssetLabels, len(al))
 
 	for label, value := range al {
 		clone[label] = value
@@ -2392,17 +2392,15 @@ func (as *AssetSet) Clone() *AssetSet {
 	as.RLock()
 	defer as.RUnlock()
 
-	assets := map[string]Asset{}
+	assets := make(map[string]Asset, len(as.assets))
 	for k, v := range as.assets {
 		assets[k] = v.Clone()
 	}
 
 	var props []AssetProperty
 	if as.props != nil {
-		props = []AssetProperty{}
-		for _, p := range as.props {
-			props = append(props, p)
-		}
+		props = make([]AssetProperty, len(as.props))
+		copy(props, as.props)
 	}
 
 	s := as.Start()

+ 11 - 11
pkg/kubecost/kubecost_codecs.go

@@ -421,8 +421,8 @@ func (target *AllocationSet) UnmarshalBinary(data []byte) (err error) {
 		target.allocations = nil
 	} else {
 		// --- [begin][read][map](map[string]*Allocation) ---
-		a := make(map[string]*Allocation)
 		b := buff.ReadInt() // map len
+		a := make(map[string]*Allocation, b)
 		for i := 0; i < b; i++ {
 			var k string
 			c := buff.ReadString() // read string
@@ -454,8 +454,8 @@ func (target *AllocationSet) UnmarshalBinary(data []byte) (err error) {
 		target.idleKeys = nil
 	} else {
 		// --- [begin][read][map](map[string]bool) ---
-		g := make(map[string]bool)
 		h := buff.ReadInt() // map len
+		g := make(map[string]bool, h)
 		for j := 0; j < h; j++ {
 			var kk string
 			l := buff.ReadString() // read string
@@ -745,8 +745,8 @@ func (target *Any) UnmarshalBinary(data []byte) (err error) {
 		a = nil
 	} else {
 		// --- [begin][read][map](map[string]string) ---
-		b := make(map[string]string)
 		c := buff.ReadInt() // map len
+		b := make(map[string]string, c)
 		for i := 0; i < c; i++ {
 			var k string
 			d := buff.ReadString() // read string
@@ -1046,8 +1046,8 @@ func (target *AssetSet) UnmarshalBinary(data []byte) (err error) {
 		target.assets = nil
 	} else {
 		// --- [begin][read][map](map[string]Asset) ---
-		a := make(map[string]Asset)
 		b := buff.ReadInt() // map len
+		a := make(map[string]Asset, b)
 		for i := 0; i < b; i++ {
 			var k string
 			c := buff.ReadString() // read string
@@ -1447,8 +1447,8 @@ func (target *Cloud) UnmarshalBinary(data []byte) (err error) {
 		a = nil
 	} else {
 		// --- [begin][read][map](map[string]string) ---
-		b := make(map[string]string)
 		c := buff.ReadInt() // map len
+		b := make(map[string]string, c)
 		for i := 0; i < c; i++ {
 			var k string
 			d := buff.ReadString() // read string
@@ -1622,8 +1622,8 @@ func (target *ClusterManagement) UnmarshalBinary(data []byte) (err error) {
 		a = nil
 	} else {
 		// --- [begin][read][map](map[string]string) ---
-		b := make(map[string]string)
 		c := buff.ReadInt() // map len
+		b := make(map[string]string, c)
 		for i := 0; i < c; i++ {
 			var k string
 			d := buff.ReadString() // read string
@@ -1808,8 +1808,8 @@ func (target *Disk) UnmarshalBinary(data []byte) (err error) {
 		a = nil
 	} else {
 		// --- [begin][read][map](map[string]string) ---
-		b := make(map[string]string)
 		c := buff.ReadInt() // map len
+		b := make(map[string]string, c)
 		for i := 0; i < c; i++ {
 			var k string
 			d := buff.ReadString() // read string
@@ -2038,8 +2038,8 @@ func (target *LoadBalancer) UnmarshalBinary(data []byte) (err error) {
 		d = nil
 	} else {
 		// --- [begin][read][map](map[string]string) ---
-		e := make(map[string]string)
 		f := buff.ReadInt() // map len
+		e := make(map[string]string, f)
 		for i := 0; i < f; i++ {
 			var k string
 			g := buff.ReadString() // read string
@@ -2232,8 +2232,8 @@ func (target *Network) UnmarshalBinary(data []byte) (err error) {
 		d = nil
 	} else {
 		// --- [begin][read][map](map[string]string) ---
-		e := make(map[string]string)
 		f := buff.ReadInt() // map len
+		e := make(map[string]string, f)
 		for i := 0; i < f; i++ {
 			var k string
 			g := buff.ReadString() // read string
@@ -2463,8 +2463,8 @@ func (target *Node) UnmarshalBinary(data []byte) (err error) {
 		d = nil
 	} else {
 		// --- [begin][read][map](map[string]string) ---
-		e := make(map[string]string)
 		f := buff.ReadInt() // map len
+		e := make(map[string]string, f)
 		for i := 0; i < f; i++ {
 			var k string
 			g := buff.ReadString() // read string
@@ -2689,8 +2689,8 @@ func (target *SharedAsset) UnmarshalBinary(data []byte) (err error) {
 		d = nil
 	} else {
 		// --- [begin][read][map](map[string]string) ---
-		e := make(map[string]string)
 		f := buff.ReadInt() // map len
+		e := make(map[string]string, f)
 		for i := 0; i < f; i++ {
 			var k string
 			g := buff.ReadString() // read string

+ 5 - 5
pkg/kubecost/properties.go

@@ -65,7 +65,7 @@ func (p *Properties) Clone() Properties {
 		return nil
 	}
 
-	clone := Properties{}
+	clone := make(Properties, len(*p))
 	for k, v := range *p {
 		clone[k] = v
 	}
@@ -707,8 +707,8 @@ func (p *Properties) UnmarshalBinary(data []byte) error {
 
 	// LabelProp
 	if buff.ReadUInt8() == 1 { // read nil byte
-		labels := map[string]string{}
 		length := buff.ReadInt() // read map len
+		labels := make(map[string]string, length)
 		for idx := 0; idx < length; idx++ {
 			key := buff.ReadString()
 			val := buff.ReadString()
@@ -719,8 +719,8 @@ func (p *Properties) UnmarshalBinary(data []byte) error {
 
 	// AnnotationProp
 	if buff.ReadUInt8() == 1 { // read nil byte
-		annotations := map[string]string{}
 		length := buff.ReadInt() // read map len
+		annotations := make(map[string]string, length)
 		for idx := 0; idx < length; idx++ {
 			key := buff.ReadString()
 			val := buff.ReadString()
@@ -731,11 +731,11 @@ func (p *Properties) UnmarshalBinary(data []byte) error {
 
 	// ServiceProp
 	if buff.ReadUInt8() == 1 { // read nil byte
-		services := []string{}
 		length := buff.ReadInt() // read map len
+		services := make([]string, length)
 		for idx := 0; idx < length; idx++ {
 			val := buff.ReadString()
-			services = append(services, val)
+			services[idx] = val
 		}
 		p.SetServices(services)
 	}

+ 9 - 24
pkg/kubecost/window.go

@@ -7,6 +7,8 @@ import (
 	"regexp"
 	"strconv"
 	"time"
+
+	"github.com/kubecost/cost-model/pkg/util"
 )
 
 const (
@@ -480,33 +482,16 @@ func (w Window) DurationOffset() (time.Duration, time.Duration, error) {
 	return duration, offset, nil
 }
 
-// DurationOffsetStrings returns formatted strings representing the duration and
-// offset of the window in terms of minutes; e.g. ("30m", "1m")
-// TODO check for nils in Window
+// DurationOffsetStrings returns formatted, Prometheus-compatible strings representing
+// the duration and offset of the window in terms of days, hours, minutes, or seconds;
+// e.g. ("7d", "1441m", "30m", "1s", "")
 func (w Window) DurationOffsetStrings() (string, string) {
-	durMins := int(w.Duration().Minutes())
-
-	offStr := ""
-	if w.End() != nil {
-		offMins := int(time.Now().Sub(*w.End()).Minutes())
-		if offMins > 1 {
-			offStr = fmt.Sprintf("%dm", int(offMins))
-		} else if offMins < -1 {
-			durMins += offMins
-		}
-	}
-
-	// default to formatting in terms of minutes
-	durStr := fmt.Sprintf("%dm", durMins)
-	if (durMins >= minutesPerDay) && (durMins%minutesPerDay == 0) {
-		// convert to days
-		durStr = fmt.Sprintf("%dd", durMins/minutesPerDay)
-	} else if (durMins >= minutesPerHour) && (durMins%minutesPerHour == 0) {
-		// convert to hours
-		durStr = fmt.Sprintf("%dh", durMins/minutesPerHour)
+	dur, off, err := w.DurationOffset()
+	if err != nil {
+		return "", ""
 	}
 
-	return durStr, offStr
+	return util.DurationOffsetStrings(dur, off)
 }
 
 type BoundaryError struct {

+ 54 - 0
pkg/util/time.go

@@ -7,9 +7,21 @@ import (
 )
 
 const (
+	// SecsPerMin expresses the amount of seconds in a minute
+	SecsPerMin = 60.0
+
+	// SecsPerHour expresses the amount of seconds in a minute
+	SecsPerHour = 3600.0
+
+	// SecsPerDay expressed the amount of seconds in a day
+	SecsPerDay = 86400.0
+
 	// MinsPerHour expresses the amount of minutes in an hour
 	MinsPerHour = 60.0
 
+	// MinsPerDay expresses the amount of minutes in a day
+	MinsPerDay = 1440.0
+
 	// HoursPerDay expresses the amount of hours in a day
 	HoursPerDay = 24.0
 
@@ -20,6 +32,48 @@ const (
 	DaysPerMonth = 30.42
 )
 
+// 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) {
+	durSecs := int64(duration.Seconds())
+	offSecs := int64(offset.Seconds())
+
+	durStr := ""
+	if durSecs > 0 {
+		if durSecs%SecsPerDay == 0 {
+			// convert to days
+			durStr = fmt.Sprintf("%dd", durSecs/SecsPerDay)
+		} else if durSecs%SecsPerHour == 0 {
+			// convert to hours
+			durStr = fmt.Sprintf("%dh", durSecs/SecsPerHour)
+		} else if durSecs%SecsPerMin == 0 {
+			// convert to mins
+			durStr = fmt.Sprintf("%dm", durSecs/SecsPerMin)
+		} else if durSecs > 0 {
+			// default to mins, as long as duration is positive
+			durStr = fmt.Sprintf("%ds", durSecs)
+		}
+	}
+
+	offStr := ""
+	if offSecs > 0 {
+		if offSecs%SecsPerDay == 0 {
+			// convert to days
+			offStr = fmt.Sprintf("%dd", offSecs/SecsPerDay)
+		} else if offSecs%SecsPerHour == 0 {
+			// convert to hours
+			offStr = fmt.Sprintf("%dh", offSecs/SecsPerHour)
+		} else if offSecs%SecsPerMin == 0 {
+			// convert to mins
+			offStr = fmt.Sprintf("%dm", offSecs/SecsPerMin)
+		} else if offSecs > 0 {
+			// default to mins, as long as offation is positive
+			offStr = fmt.Sprintf("%ds", offSecs)
+		}
+	}
+	return durStr, offStr
+}
+
 // ParseDuration converts a Prometheus-style duration string into a Duration
 func ParseDuration(duration string) (*time.Duration, error) {
 	unitStr := duration[len(duration)-1:]

+ 56 - 0
pkg/util/time_test.go

@@ -0,0 +1,56 @@
+package util
+
+import (
+	"testing"
+	"time"
+)
+
+func TestDurationOffsetStrings(t *testing.T) {
+	dur, off := "", ""
+
+	dur, off = DurationOffsetStrings(0, 0)
+	if dur != "" || off != "" {
+		t.Fatalf("DurationOffsetStrings: exp (%s %s); act (%s, %s)", "", "", dur, off)
+	}
+
+	dur, off = DurationOffsetStrings(24*time.Hour, 0)
+	if dur != "1d" || off != "" {
+		t.Fatalf("DurationOffsetStrings: exp (%s %s); act (%s, %s)", "1d", "", dur, off)
+	}
+
+	dur, off = DurationOffsetStrings(24*time.Hour+5*time.Minute, 0)
+	if dur != "1445m" || off != "" {
+		t.Fatalf("DurationOffsetStrings: exp (%s %s); act (%s, %s)", "1445m", "", dur, off)
+	}
+
+	dur, off = DurationOffsetStrings(25*time.Hour, 5*time.Minute)
+	if dur != "25h" || off != "5m" {
+		t.Fatalf("DurationOffsetStrings: exp (%s %s); act (%s, %s)", "25h", "5m", dur, off)
+	}
+
+	dur, off = DurationOffsetStrings(25*time.Hour, 60*time.Minute)
+	if dur != "25h" || off != "1h" {
+		t.Fatalf("DurationOffsetStrings: exp (%s %s); act (%s, %s)", "25h", "1h", dur, off)
+	}
+
+	dur, off = DurationOffsetStrings(72*time.Hour, 1440*time.Minute)
+	if dur != "3d" || off != "1d" {
+		t.Fatalf("DurationOffsetStrings: exp (%s %s); act (%s, %s)", "3d", "1d", dur, off)
+	}
+
+	dur, off = DurationOffsetStrings(25*time.Hour, 1*time.Second)
+	if dur != "25h" || off != "1s" {
+		t.Fatalf("DurationOffsetStrings: exp (%s %s); act (%s, %s)", "25h", "1s", dur, off)
+	}
+
+	dur, off = DurationOffsetStrings(24*time.Hour+time.Second, 1*time.Second)
+	if dur != "86401s" || off != "1s" {
+		t.Fatalf("DurationOffsetStrings: exp (%s %s); act (%s, %s)", "86401s", "1s", dur, off)
+	}
+
+	// Expect empty strings if durations are negative
+	dur, off = DurationOffsetStrings(-25*time.Hour, -1*time.Second)
+	if dur != "" || off != "" {
+		t.Fatalf("DurationOffsetStrings: exp (%s %s); act (%s, %s)", "", "", dur, off)
+	}
+}