Merge develop into master (#660)

* Split window.ToDurationOffset into DurationOffset and DurationOffsetStrings

* Check and register annotations collectors if enabled.

* add csv fallback (#641) (#646)

* add csv fallback

* log class match

* add counts by source

* add test for pricing source counter and make the names of sources public

* Fix the issue with empty pod name on annotation metric.

* Only Emit label_ and annotation_ metrics if they have values!

* Simplify label and annotation metric to labels.

* Ajay tripathy pvc error fix (#644)

* add csv fallback

* log class match

* filter empty volumenames out

* Ajay tripathy fix e2custom (#623)

* pass offset to ccdr

* remove conflict

* add e2custom support

* revert cntext background change

* fix improperly named constant for govcloud lookup (#651)

* Use compatibility region implementation. (#653)

* WIP AWS idle investigation

* Fix idle allocation bug for windows < 1h; improve DurationOffset string conversion

* Commit missing test file

* Merge master into develop. (#658)

* add csv fallback (#641)

* add csv fallback

* log class match

* add counts by source

* add test for pricing source counter and make the names of sources public

* Bump to version 1.71.1

* Bump version to 1.72.0 (#659)

Co-authored-by: Niko Kovacevic <nikovacevic@gmail.com>
Co-authored-by: Matt Bolt <mbolt35@gmail.com>

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{

pkg/cloud/gcpprovider.go

@@ -733,6 +733,7 @@ func (gcp *GCP) parsePage(r io.Reader, inputKeys map[string]Key, pvKeys map[stri
 						candidateKeys = append(candidateKeys, region+","+"e2small"+","+usageType)
 						candidateKeys = append(candidateKeys, region+","+"e2medium"+","+usageType)
 						candidateKeys = append(candidateKeys, region+","+"e2standard"+","+usageType)
+						candidateKeys = append(candidateKeys, region+","+"e2custom"+","+usageType)
 					} else {
 						candidateKey := region + "," + instanceType + "," + usageType
 						candidateKeys = append(candidateKeys, candidateKey)

pkg/costmodel/aggregation.go

@@ -1294,7 +1294,7 @@ func (a *Accesses) ComputeAggregateCostModel(promClient prometheusClient.Client,
 		window.Set(&s, &e)
 	}
 
-	dur, off := window.ToDurationOffset()
+	dur, off := window.DurationOffsetStrings()
 	key := fmt.Sprintf(`%s:%s:%fh:%t`, dur, off, resolution.Hours(), remoteEnabled)
 
 	// report message about which of the two caches hit. by default report a miss
@@ -1391,22 +1391,38 @@ func (a *Accesses) ComputeAggregateCostModel(promClient prometheusClient.Client,
 
 	idleCoefficients := make(map[string]float64)
 	if allocateIdle {
-		duration, offset := window.ToDurationOffset()
-
-		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
 		}
 	}
@@ -1540,7 +1556,7 @@ func GenerateAggKey(window kubecost.Window, field string, subfields []string, op
 
 	// Covert to duration, offset so that cache hits occur, even when timestamps have
 	// shifted slightly.
-	duration, offset := window.ToDurationOffset()
+	duration, offset := window.DurationOffsetStrings()
 
 	// parse, trim, and sort podprefix filters
 	podPrefixFilters := []string{}

pkg/costmodel/costmodel.go

@@ -177,10 +177,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) by (persistentvolumeclaim, storageclass, namespace, volumename, cluster_id)
+	queryPVRequestsStr = `avg(avg(kube_persistentvolumeclaim_info{volumename != ""}) by (persistentvolumeclaim, storageclass, namespace, volumename, cluster_id)
 	*
 	on (persistentvolumeclaim, namespace, cluster_id) group_right(storageclass, volumename)
-	sum(kube_persistentvolumeclaim_resource_requests_storage_bytes) by (persistentvolumeclaim, namespace, cluster_id, kubernetes_name)) by (persistentvolumeclaim, storageclass, namespace, volumename, cluster_id)`
+	sum(kube_persistentvolumeclaim_resource_requests_storage_bytes{volumename != ""}) by (persistentvolumeclaim, namespace, cluster_id, kubernetes_name)) by (persistentvolumeclaim, storageclass, namespace, volumename, cluster_id)`
 	// 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
@@ -1989,7 +1989,7 @@ func (cm *CostModel) costDataRange(cli prometheusClient.Client, cp costAnalyzerC
 	}
 
 	if window.Minutes() > 0 {
-		dur, off := window.ToDurationOffset()
+		dur, off := window.DurationOffsetStrings()
 		err = findDeletedNodeInfo(cli, missingNodes, dur, off)
 		if err != nil {
 			klog.V(1).Infof("Error fetching historical node data: %s", err.Error())

pkg/costmodel/metrics.go

@@ -9,9 +9,11 @@ import (
 
 	"github.com/kubecost/cost-model/pkg/cloud"
 	"github.com/kubecost/cost-model/pkg/clustercache"
+	"github.com/kubecost/cost-model/pkg/env"
 	"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"
@@ -42,8 +44,10 @@ func (sc StatefulsetCollector) Collect(ch chan<- prometheus.Metric) {
 	ds := sc.KubeClusterCache.GetAllStatefulSets()
 	for _, statefulset := range ds {
 		labels, values := prom.KubeLabelsToLabels(statefulset.Spec.Selector.MatchLabels)
-		m := newStatefulsetMetric(statefulset.GetName(), statefulset.GetNamespace(), "statefulSet_match_labels", labels, values)
-		ch <- m
+		if len(labels) > 0 {
+			m := newStatefulsetMetric(statefulset.GetName(), statefulset.GetNamespace(), "statefulSet_match_labels", labels, values)
+			ch <- m
+		}
 	}
 }
 
@@ -128,8 +132,10 @@ func (sc DeploymentCollector) Collect(ch chan<- prometheus.Metric) {
 	ds := sc.KubeClusterCache.GetAllDeployments()
 	for _, deployment := range ds {
 		labels, values := prom.KubeLabelsToLabels(deployment.Spec.Selector.MatchLabels)
-		m := newDeploymentMetric(deployment.GetName(), deployment.GetNamespace(), "deployment_match_labels", labels, values)
-		ch <- m
+		if len(labels) > 0 {
+			m := newDeploymentMetric(deployment.GetName(), deployment.GetNamespace(), "deployment_match_labels", labels, values)
+			ch <- m
+		}
 	}
 }
 
@@ -214,8 +220,10 @@ func (sc ServiceCollector) Collect(ch chan<- prometheus.Metric) {
 	svcs := sc.KubeClusterCache.GetAllServices()
 	for _, svc := range svcs {
 		labels, values := prom.KubeLabelsToLabels(svc.Spec.Selector)
-		m := newServiceMetric(svc.GetName(), svc.GetNamespace(), "service_selector_labels", labels, values)
-		ch <- m
+		if len(labels) > 0 {
+			m := newServiceMetric(svc.GetName(), svc.GetNamespace(), "service_selector_labels", labels, values)
+			ch <- m
+		}
 	}
 }
 
@@ -300,8 +308,10 @@ func (nsac NamespaceAnnotationCollector) Collect(ch chan<- prometheus.Metric) {
 	namespaces := nsac.KubeClusterCache.GetAllNamespaces()
 	for _, namespace := range namespaces {
 		labels, values := prom.KubeAnnotationsToLabels(namespace.Annotations)
-		m := newNamespaceAnnotationsMetric(namespace.GetName(), "kube_namespace_annotations", labels, values)
-		ch <- m
+		if len(labels) > 0 {
+			m := newNamespaceAnnotationsMetric(namespace.GetName(), "kube_namespace_annotations", labels, values)
+			ch <- m
+		}
 	}
 }
 
@@ -380,8 +390,10 @@ func (pac PodAnnotationCollector) Collect(ch chan<- prometheus.Metric) {
 	pods := pac.KubeClusterCache.GetAllPods()
 	for _, pod := range pods {
 		labels, values := prom.KubeAnnotationsToLabels(pod.Annotations)
-		m := newPodAnnotationMetric(pod.GetNamespace(), pod.GetName(), "kube_pod_annotations", labels, values)
-		ch <- m
+		if len(labels) > 0 {
+			m := newPodAnnotationMetric(pod.GetNamespace(), pod.GetName(), "kube_pod_annotations", labels, values)
+			ch <- m
+		}
 	}
 }
 
@@ -403,6 +415,7 @@ type PodAnnotationsMetric struct {
 func newPodAnnotationMetric(namespace, name, fqname string, labelNames []string, labelValues []string) PodAnnotationsMetric {
 	return PodAnnotationsMetric{
 		namespace:   namespace,
+		name:        name,
 		fqName:      fqname,
 		labelNames:  labelNames,
 		labelValues: labelValues,
@@ -644,6 +657,18 @@ func initCostModelMetrics(clusterCache clustercache.ClusterCache, provider cloud
 			KubeClientSet: clusterCache.GetClient(),
 			Cloud:         provider,
 		})
+
+		if env.IsEmitNamespaceAnnotationsMetric() {
+			prometheus.MustRegister(NamespaceAnnotationCollector{
+				KubeClusterCache: clusterCache,
+			})
+		}
+
+		if env.IsEmitPodAnnotationsMetric() {
+			prometheus.MustRegister(PodAnnotationCollector{
+				KubeClusterCache: clusterCache,
+			})
+		}
 	})
 }
 
@@ -766,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 {

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.1")
+	return Get(AppVersionEnvVar, "1.72.0")
 }
 
 // IsEmitNamespaceAnnotationsMetric returns true if cost-model is configured to emit the kube_namespace_annotations metric

pkg/kubecost/status.go

@@ -5,6 +5,7 @@ import "time"
 // ETLStatus describes ETL metadata
 type ETLStatus struct {
 	Coverage    Window           `json:"coverage"`
+	LastRun     time.Time        `json:"lastRun"`
 	Progress    float64          `json:"progress"`
 	RefreshRate string           `json:"refreshRate"`
 	StartTime   time.Time        `json:"startTime"`

pkg/kubecost/window.go

@@ -7,6 +7,8 @@ import (
 	"regexp"
 	"strconv"
 	"time"
+
+	"github.com/kubecost/cost-model/pkg/util"
 )
 
 const (
@@ -467,32 +469,29 @@ func (w Window) String() string {
 	return fmt.Sprintf("[%s, %s)", w.start.Format("2006-01-02T15:04:05-0700"), w.end.Format("2006-01-02T15:04:05-0700"))
 }
 
-// ToDurationOffset returns formatted strings representing the duration and
-// offset of the window in terms of minutes; e.g. ("30m", "1m")
-func (w Window) ToDurationOffset() (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
-		}
+// DurationOffset returns durations representing the duration and offset of the
+// given window
+func (w Window) DurationOffset() (time.Duration, time.Duration, error) {
+	if w.IsOpen() || w.IsNegative() {
+		return 0, 0, fmt.Errorf("illegal window: %s", w)
 	}
 
-	// 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)
+	duration := w.Duration()
+	offset := time.Now().Sub(*w.End())
+
+	return duration, offset, nil
+}
+
+// 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) {
+	dur, off, err := w.DurationOffset()
+	if err != nil {
+		return "", ""
 	}
 
-	return durStr, offStr
+	return util.DurationOffsetStrings(dur, off)
 }
 
 type BoundaryError struct {

pkg/kubecost/window_test.go

@@ -566,12 +566,12 @@ func TestParseWindowWithOffsetString(t *testing.T) {
 // TODO niko/etl
 // func TestWindow_String(t *testing.T) {}
 
-func TestWindow_ToDurationOffset(t *testing.T) {
+func TestWindow_DurationOffsetStrings(t *testing.T) {
 	w, err := ParseWindowUTC("1d")
 	if err != nil {
 		t.Fatalf(`unexpected error parsing "1d": %s`, err)
 	}
-	dur, off := w.ToDurationOffset()
+	dur, off := w.DurationOffsetStrings()
 	if dur != "1d" {
 		t.Fatalf(`expect: window to be "1d"; actual: "%s"`, dur)
 	}
@@ -583,7 +583,7 @@ func TestWindow_ToDurationOffset(t *testing.T) {
 	if err != nil {
 		t.Fatalf(`unexpected error parsing "1d": %s`, err)
 	}
-	dur, off = w.ToDurationOffset()
+	dur, off = w.DurationOffsetStrings()
 	if dur != "3h" {
 		t.Fatalf(`expect: window to be "3h"; actual: "%s"`, dur)
 	}
@@ -595,7 +595,7 @@ func TestWindow_ToDurationOffset(t *testing.T) {
 	if err != nil {
 		t.Fatalf(`unexpected error parsing "1d": %s`, err)
 	}
-	dur, off = w.ToDurationOffset()
+	dur, off = w.DurationOffsetStrings()
 	if dur != "10m" {
 		t.Fatalf(`expect: window to be "10m"; actual: "%s"`, dur)
 	}
@@ -607,7 +607,7 @@ func TestWindow_ToDurationOffset(t *testing.T) {
 	if err != nil {
 		t.Fatalf(`unexpected error parsing "1589448338,1589534798": %s`, err)
 	}
-	dur, off = w.ToDurationOffset()
+	dur, off = w.DurationOffsetStrings()
 	if dur != "1441m" {
 		t.Fatalf(`expect: window to be "1441m"; actual: "%s"`, dur)
 	}
@@ -619,7 +619,7 @@ func TestWindow_ToDurationOffset(t *testing.T) {
 	if err != nil {
 		t.Fatalf(`unexpected error parsing "1589448338,1589534798": %s`, err)
 	}
-	dur, off = w.ToDurationOffset()
+	dur, off = w.DurationOffsetStrings()
 	if dur != "1d" {
 		t.Fatalf(`expect: window to be "1d"; actual: "%s"`, dur)
 	}

pkg/prom/metrics.go

@@ -72,36 +72,31 @@ func LabelNamesFrom(labels map[string]string) []string {
 	return keys
 }
 
-// Converts kubernetes labels into prometheus labels.
-func KubeLabelsToLabels(labels map[string]string) ([]string, []string) {
-	labelKeys := make([]string, 0, len(labels))
-	for k := range labels {
-		labelKeys = append(labelKeys, k)
+// Prepends a qualifier string to the keys provided in the m map and returns the new keys and values.
+func KubePrependQualifierToLabels(m map[string]string, qualifier string) ([]string, []string) {
+	keys := make([]string, 0, len(m))
+	for k := range m {
+		keys = append(keys, k)
 	}
-	sort.Strings(labelKeys)
+	sort.Strings(keys)
 
-	labelValues := make([]string, 0, len(labels))
-	for i, k := range labelKeys {
-		labelKeys[i] = "label_" + SanitizeLabelName(k)
-		labelValues = append(labelValues, labels[k])
+	values := make([]string, 0, len(m))
+	for i, k := range keys {
+		keys[i] = qualifier + SanitizeLabelName(k)
+		values = append(values, m[k])
 	}
-	return labelKeys, labelValues
+
+	return keys, values
+}
+
+// Converts kubernetes labels into prometheus labels.
+func KubeLabelsToLabels(labels map[string]string) ([]string, []string) {
+	return KubePrependQualifierToLabels(labels, "label_")
 }
 
 // Converts kubernetes annotations into prometheus labels.
 func KubeAnnotationsToLabels(labels map[string]string) ([]string, []string) {
-	labelKeys := make([]string, 0, len(labels))
-	for k := range labels {
-		labelKeys = append(labelKeys, k)
-	}
-	sort.Strings(labelKeys)
-
-	labelValues := make([]string, 0, len(labels))
-	for i, k := range labelKeys {
-		labelKeys[i] = "annotation_" + SanitizeLabelName(k)
-		labelValues = append(labelValues, labels[k])
-	}
-	return labelKeys, labelValues
+	return KubePrependQualifierToLabels(labels, "annotation_")
 }
 
 // Replaces all illegal prometheus label characters with _

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:]

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)
+	}
+}