You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by mi...@apache.org on 2020/04/21 20:30:01 UTC
[trafficcontrol] branch master updated: Dsstats rename (#4643)
This is an automated email from the ASF dual-hosted git repository.
mitchell852 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git
The following commit(s) were added to refs/heads/master by this push:
new f01019a Dsstats rename (#4643)
f01019a is described below
commit f01019a7c72bae1212fc2f08f31177305a58741c
Author: ocket8888 <oc...@apache.org>
AuthorDate: Tue Apr 21 14:29:53 2020 -0600
Dsstats rename (#4643)
* renamed the ds_stats return property
* Updated TP to use new field name
* Updated docs for new field
* Fixed api2.x being designated legacy and vice-versa
* Updated CHANGELOG
---
CHANGELOG.md | 1 +
docs/source/api/v1/deliveryservice_stats.rst | 2 +
docs/source/api/v2/deliveryservice_stats.rst | 14 +--
lib/go-tc/traffic_stats.go | 22 +++-
.../trafficstats/deliveryservice.go | 129 ++++++++++++++++-----
.../common/modules/chart/bps/chart.bps.tpl.html | 2 +-
6 files changed, 125 insertions(+), 45 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 618122b..d113b26 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -54,6 +54,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
- Updated Traffic Monitor to default to polling both IPv4 and IPv6.
- Traffic Ops, Traffic Monitor, Traffic Stats, and Grove are now compiled using Go version 1.14. This requires a Traffic Vault config update (see note below).
- Existing installations **must** enable TLSv1.1 for Traffic Vault in order for Traffic Ops to reach it. See [Enabling TLS 1.1](https://traffic-control-cdn.readthedocs.io/en/latest/admin/traffic_vault.html#tv-admin-enable-tlsv1-1) in the Traffic Vault administrator's guide for instructions.
+- Changed the `totalBytes` property of responses to GET requests to `/deliveryservice_stats` to the more appropriate `totalKiloBytes` in API 2.x
### Deprecated/Removed
- The Traffic Ops `db/admin.pl` script has now been removed. Please use the `db/admin` binary instead.
diff --git a/docs/source/api/v1/deliveryservice_stats.rst b/docs/source/api/v1/deliveryservice_stats.rst
index 464a85a..d042d6f 100644
--- a/docs/source/api/v1/deliveryservice_stats.rst
+++ b/docs/source/api/v1/deliveryservice_stats.rst
@@ -21,6 +21,8 @@
*************************
.. versionadded:: 1.2
+.. danger:: The output of this endpoint can be confusing and/or misleading. Specifically, the response field ``totalBytes`` does **not** contain an amount in units of Bytes. See the field description for more information, or use :ref:`to-api-v2-deliveryservice_stats` for more intuitive response objects.
+
``GET``
=======
Retrieves time-aggregated statistics on a specific :term:`Delivery Service`.
diff --git a/docs/source/api/v2/deliveryservice_stats.rst b/docs/source/api/v2/deliveryservice_stats.rst
index d3b6c28..b8c494d 100644
--- a/docs/source/api/v2/deliveryservice_stats.rst
+++ b/docs/source/api/v2/deliveryservice_stats.rst
@@ -120,11 +120,6 @@ Response Structure
:time: The time at which the measurement was taken. This corresponds to the *beginning* of the interval. This time comes in the format of either an :rfc:`3339`-formatted string, or a number containing the number of nanoseconds since the Unix Epoch depending on the "Accept" header sent by the client, according to the rules outlined in `Content Format`_.
:value: The value of the requested ``metricType`` at the time given by ``time``. This will always be a floating point number, unless no data is available for the data interval, in which case it will be ``null``
-:source: A legacy field meant only for plugins that override this endpoint to name themselves. Should always be "TrafficStats".
-
- .. deprecated:: 1.4
- As this has no known purpose, developers are advised it will be removed in the future.
-
:summary: An object containing summary statistics describing the data series
:average: The arithmetic mean of the data's values
@@ -134,14 +129,9 @@ Response Structure
:min: The minimum value that can be found in the requested data set
:ninetyEighthPercentile: Data points with values greater than or equal to this number constitute the "top" 2% of the data set
:ninetyFifthPercentile: Data points with values greater than or equal to this number constitute the "top" 5% of the data set
- :totalBytes: When the ``metricType`` requested is ``kbps``, this will contain the total number of bytes transferred by the :term:`Delivery Service` within the requested time window. Note that fractional amounts are possible, as the data transfer rate will almost certainly not be cleanly divided by the requested time range.
+ :totalKiloBytes: When the ``metricType`` requested is ``kbps``, this will contain the total number of kilobytes transferred by the :term:`Delivery Service` within the requested time window. Note that fractional amounts are possible, as the data transfer rate will almost certainly not be cleanly divided by the requested time range.
:totalTransactions: When the ``metricType`` requested is **not** ``kbps``, this will contain the total number of transactions completed by the :term:`Delivery Service` within the requested time window. Note that fractional amounts are possible, as the transaction rate will almost certainly not be cleanly divided by the requested time range.
-:version: A legacy field that seems to have been meant to indicate the API version used. Will always be "1.2"
-
- .. deprecated:: 1.4
- As this has no known purpose, developers are advised it will be removed in the future.
-
.. code-block:: http
:caption: Response Example
@@ -189,7 +179,7 @@ Response Structure
"min": 0,
"ninetyEighthPercentile": 0,
"ninetyFifthPercentile": 0,
- "totalBytes": null,
+ "totalKiloBytes": null,
"totalTransactions": 0
},
"version": "1.2"
diff --git a/lib/go-tc/traffic_stats.go b/lib/go-tc/traffic_stats.go
index 7932c8c..aad075d 100644
--- a/lib/go-tc/traffic_stats.go
+++ b/lib/go-tc/traffic_stats.go
@@ -163,7 +163,10 @@ func (c *TrafficStatsConfig) OffsetString() string {
// deliveryservice_stats "Traffic Stats" endpoints.
// It contains the deprecated, legacy fields "Source" and "Version"
type TrafficDSStatsResponseV1 struct {
- TrafficDSStatsResponse
+ // Series holds the actual data - it is NOT in general the same as a github.com/influxdata/influxdb1-client/models.Row
+ Series *TrafficStatsSeries `json:"series,omitempty"`
+ // Summary contains summary statistics of the data in Series
+ Summary *LegacyTrafficDSStatsSummary `json:"summary,omitempty"`
// Source has an unknown purpose. I believe this is supposed to name the "plugin" that provided
// the data - kept for compatibility with the Perl version(s) of the "Traffic Stats endpoints".
Source string `json:"source"`
@@ -204,13 +207,26 @@ type TrafficStatsSummary struct {
NinetyFifthPercentile float64 `json:"ninetyFifthPercentile"`
}
+type LegacyTrafficDSStatsSummary struct {
+ TrafficStatsSummary
+ // TotalBytes is the total number of kilobytes served when the "metric type" requested is "kbps"
+ // (or actually just contains "kbps"). If this is not nil, TotalTransactions *should* always be
+ // nil.
+ TotalBytes *float64 `json:"totalBytes"`
+ // Totaltransactions is the total number of transactions within the requested window. Whenever
+ // the requested metric doesn't contain "kbps", it assumed to be some kind of transactions
+ // measurement. In that case, this will not be nil - otherwise it will be nil. If this not nil,
+ // TotalBytes *should* always be nil.
+ TotalTransactions *float64 `json:"totalTransactions"`
+}
+
// TrafficDSStatsSummary contains summary statistics for a data series for deliveryservice stats.
type TrafficDSStatsSummary struct {
TrafficStatsSummary
- // TotalBytes is the total number of bytes served when the "metric type" requested is "kbps"
+ // TotalKiloBytes is the total number of kilobytes served when the "metric type" requested is "kbps"
// (or actually just contains "kbps"). If this is not nil, TotalTransactions *should* always be
// nil.
- TotalBytes *float64 `json:"totalBytes"`
+ TotalKiloBytes *float64 `json:"totalKiloBytes"`
// Totaltransactions is the total number of transactions within the requested window. Whenever
// the requested metric doesn't contain "kbps", it assumed to be some kind of transactions
// measurement. In that case, this will not be nil - otherwise it will be nil. If this not nil,
diff --git a/traffic_ops/traffic_ops_golang/trafficstats/deliveryservice.go b/traffic_ops/traffic_ops_golang/trafficstats/deliveryservice.go
index 82d547d..066b505 100644
--- a/traffic_ops/traffic_ops_golang/trafficstats/deliveryservice.go
+++ b/traffic_ops/traffic_ops_golang/trafficstats/deliveryservice.go
@@ -185,44 +185,52 @@ func GetDSStats(w http.ResponseWriter, r *http.Request) {
return
}
- resp := tc.TrafficDSStatsResponseV1{
- Source: tc.TRAFFIC_STATS_SOURCE,
- Version: tc.TRAFFIC_STATS_VERSION,
+ if inf.Version.Major > 1 {
+ handleRequest(w, r, client, c, inf)
+ } else {
+ handleLegacyRequest(w, r, client, c, inf)
}
+}
+func handleRequest(w http.ResponseWriter, r *http.Request, client *influx.Client, cfg tc.TrafficDSStatsConfig, inf *api.APIInfo) {
// TODO: as above, this could be done on TO itself, thus sending only one synchronous request
// per hit on this endpoint, rather than the current two. Not sure if that's worth it for large
// data sets, though.
- if !c.ExcludeSummary {
- summary, err := getDSSummary(client, &c, inf.Config.ConfigInflux.DSDBName)
+ var resp tc.TrafficDSStatsResponse
+ if !cfg.ExcludeSummary {
+ summary, kBs, txns, err := getDSSummary(client, &cfg, inf.Config.ConfigInflux.DSDBName)
if err != nil {
- sysErr = fmt.Errorf("Getting summary response from Influx: %v", err)
- api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, sysErr)
+ sysErr := fmt.Errorf("Getting summary response from Influx: %v", err)
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, sysErr)
return
}
// match Perl implementation and set summary to zero values if no data
if summary != nil {
- resp.Summary = summary
+ resp.Summary = &tc.TrafficDSStatsSummary{
+ TrafficStatsSummary: *summary,
+ TotalKiloBytes: kBs,
+ TotalTransactions: txns,
+ }
} else {
resp.Summary = &tc.TrafficDSStatsSummary{}
}
}
- if !c.ExcludeSeries {
- series, err := getDSSeries(client, &c, inf.Config.ConfigInflux.DSDBName)
+ if !cfg.ExcludeSeries {
+ series, err := getDSSeries(client, &cfg, inf.Config.ConfigInflux.DSDBName)
if err != nil {
- sysErr = fmt.Errorf("Getting summary response from Influx: %v", err)
- api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, sysErr)
+ sysErr := fmt.Errorf("Getting summary response from Influx: %v", err)
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, sysErr)
return
}
// match Perl implementation and omit series if no data
if series != nil {
- if !c.Unix {
+ if !cfg.Unix {
series.FormatTimestamps()
}
@@ -233,21 +241,85 @@ func GetDSStats(w http.ResponseWriter, r *http.Request) {
var respObj struct {
Response interface{} `json:"response"`
}
- if inf.Version.Major > 1 {
- respObj.Response = resp.TrafficDSStatsResponse
+ respObj.Response = resp
+
+ respBts, err := json.Marshal(respObj)
+ if err != nil {
+ sysErr := fmt.Errorf("Marshalling response: %v", err)
+ errCode := http.StatusInternalServerError
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, nil, sysErr)
+ return
+ }
+
+ if cfg.Unix {
+ w.Header().Set(rfc.ContentType, jsonWithUnixTimestamps.String())
} else {
- respObj.Response = resp
+ w.Header().Set(rfc.ContentType, jsonWithRFCTimestamps.String())
}
+ w.Header().Set(http.CanonicalHeaderKey("vary"), http.CanonicalHeaderKey("Accept"))
+ w.Write(append(respBts, '\n'))
+}
+
+func handleLegacyRequest(w http.ResponseWriter, r *http.Request, client *influx.Client, cfg tc.TrafficDSStatsConfig, inf *api.APIInfo) {
+ // TODO: as above, this could be done on TO itself, thus sending only one synchronous request
+ // per hit on this endpoint, rather than the current two. Not sure if that's worth it for large
+ // data sets, though.
+ var resp tc.TrafficDSStatsResponseV1
+ if !cfg.ExcludeSummary {
+ summary, kBs, txns, err := getDSSummary(client, &cfg, inf.Config.ConfigInflux.DSDBName)
+
+ if err != nil {
+ sysErr := fmt.Errorf("Getting summary response from Influx: %v", err)
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, sysErr)
+ return
+ }
+
+ // match Perl implementation and set summary to zero values if no data
+ if summary != nil {
+ resp.Summary = &tc.LegacyTrafficDSStatsSummary{
+ TrafficStatsSummary: *summary,
+ TotalBytes: kBs,
+ TotalTransactions: txns,
+ }
+ } else {
+ resp.Summary = &tc.LegacyTrafficDSStatsSummary{}
+ }
+
+ }
+
+ if !cfg.ExcludeSeries {
+ series, err := getDSSeries(client, &cfg, inf.Config.ConfigInflux.DSDBName)
+
+ if err != nil {
+ sysErr := fmt.Errorf("Getting summary response from Influx: %v", err)
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, sysErr)
+ return
+ }
+
+ // match Perl implementation and omit series if no data
+ if series != nil {
+ if !cfg.Unix {
+ series.FormatTimestamps()
+ }
+
+ resp.Series = series
+ }
+ }
+
+ var respObj struct {
+ Response interface{} `json:"response"`
+ }
+ respObj.Response = resp
respBts, err := json.Marshal(respObj)
if err != nil {
- sysErr = fmt.Errorf("Marshalling response: %v", err)
- errCode = http.StatusInternalServerError
- api.HandleErr(w, r, tx, errCode, nil, sysErr)
+ sysErr := fmt.Errorf("Marshalling response: %v", err)
+ errCode := http.StatusInternalServerError
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, nil, sysErr)
return
}
- if c.Unix {
+ if cfg.Unix {
w.Header().Set(rfc.ContentType, jsonWithUnixTimestamps.String())
} else {
w.Header().Set(rfc.ContentType, jsonWithRFCTimestamps.String())
@@ -256,8 +328,7 @@ func GetDSStats(w http.ResponseWriter, r *http.Request) {
w.Write(append(respBts, '\n'))
}
-func getDSSummary(client *influx.Client, conf *tc.TrafficDSStatsConfig, db string) (*tc.TrafficDSStatsSummary, error) {
- s := tc.TrafficDSStatsSummary{}
+func getDSSummary(client *influx.Client, conf *tc.TrafficDSStatsConfig, db string) (*tc.TrafficStatsSummary, *float64, *float64, error) {
qStr := fmt.Sprintf(dsSummaryQuery, db, conf.MetricType)
q := influx.NewQueryWithParameters(qStr,
db,
@@ -270,21 +341,21 @@ func getDSSummary(client *influx.Client, conf *tc.TrafficDSStatsConfig, db strin
})
ts, err := getSummary(db, q, client)
if err != nil || ts == nil {
- return nil, err
+ return nil, nil, nil, err
}
- s.TrafficStatsSummary = *ts
-
- value := float64(s.Count*60) * s.Average
+ var totalKB *float64
+ var totalTXN *float64
+ value := float64(ts.Count*60) * ts.Average
if conf.MetricType == "kbps" {
// TotalBytes is actually in units of kB....
value /= 8
- s.TotalBytes = &value
+ totalKB = &value
} else {
- s.TotalTransactions = &value
+ totalTXN = &value
}
- return &s, nil
+ return ts, totalKB, totalTXN, nil
}
func dsTenantIDFromXMLID(xmlid string, tx *sql.Tx) (bool, uint, error) {
diff --git a/traffic_portal/app/src/common/modules/chart/bps/chart.bps.tpl.html b/traffic_portal/app/src/common/modules/chart/bps/chart.bps.tpl.html
index 3a4adfd..1601e65 100644
--- a/traffic_portal/app/src/common/modules/chart/bps/chart.bps.tpl.html
+++ b/traffic_portal/app/src/common/modules/chart/bps/chart.bps.tpl.html
@@ -37,7 +37,7 @@ under the License.
<tbody>
<tr>
<td>Delivered</td>
- <td class="fs15 fw700 text-right total">{{summaryData.totalBytes | unitsFilter:true}}</td>
+ <td class="fs15 fw700 text-right total">{{summaryData.totalKiloBytes | unitsFilter:true}}</td>
</tr>
<tr>
<td>95th percentile</td>