Skip to content
This repository was archived by the owner on Nov 24, 2025. It is now read-only.

Fixes Stats Summary V5 apis to respond with RFC3339 date/time Format#7545

Merged
srijeet0406 merged 23 commits intoapache:masterfrom
jagan-parthiban:improve/rfc3339-stats-summary
Jul 5, 2023
Merged

Fixes Stats Summary V5 apis to respond with RFC3339 date/time Format#7545
srijeet0406 merged 23 commits intoapache:masterfrom
jagan-parthiban:improve/rfc3339-stats-summary

Conversation

@jagan-parthiban
Copy link
Copy Markdown
Contributor

Closes: #7544
Related: #5911


  • Documentation
  • Traffic Ops

What is the best way to verify this PR?

Make Api calls to stats_summary 5.0

Stats_Summary to Use RFC3339 Format

curl --request GET \
  --url https://localhost:8443/api/5.0/stats_summary


{
	"response": [
		{
			"statDate": "2023-03-09",
			"summaryTime": "2023-03-09T00:59:12+05:30",
			"cdnName": "cdn101",
			"deliveryServiceName": "all",
			"statName": "daily_maxgbps",
			"statValue": 101
		}
	]
}

If this is a bugfix, which Traffic Control versions contained the bug?

  • 7.0.1

PR submission checklist

@jagan-parthiban jagan-parthiban changed the title Fixes Stats SUmmary V5 apis to respond with RFC3339 date/time Format Fixes Stats Summary V5 apis to respond with RFC3339 date/time Format May 28, 2023
@jagan-parthiban jagan-parthiban marked this pull request as ready for review May 30, 2023 13:01
@ocket8888 ocket8888 added Traffic Ops related to Traffic Ops low impact affects only a small portion of a CDN, and cannot itself break one tech debt rework due to choosing easy/limited solution labels May 31, 2023
Comment thread traffic_ops/traffic_ops_golang/trafficstats/stats_summary.go Fixed
@codecov
Copy link
Copy Markdown

codecov Bot commented May 31, 2023

Codecov Report

Merging #7545 (2c7b7d4) into master (f924716) will increase coverage by 2.68%.
The diff coverage is 16.99%.

@@             Coverage Diff              @@
##             master    #7545      +/-   ##
============================================
+ Coverage     27.51%   30.20%   +2.68%     
  Complexity       98       98              
============================================
  Files           686      793     +107     
  Lines         78851    83815    +4964     
  Branches         90      896     +806     
============================================
+ Hits          21697    25315    +3618     
- Misses        55090    56374    +1284     
- Partials       2064     2126      +62     
Flag Coverage Δ
golib_unit 48.30% <0.00%> (-0.30%) ⬇️
grove_unit 4.60% <ø> (ø)
t3c_unit 5.28% <ø> (ø)
traffic_monitor_unit 21.28% <ø> (ø)
traffic_ops_integration 69.42% <100.00%> (ø)
traffic_ops_unit 23.09% <28.76%> (+0.02%) ⬆️
traffic_portal_v2 74.35% <ø> (?)
traffic_stats_unit 10.14% <ø> (ø)
unit_tests 27.32% <14.18%> (+3.13%) ⬆️
v3 57.79% <ø> (ø)
v4 79.18% <ø> (ø)
v5 78.63% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
lib/go-tc/stats_summary.go 0.00% <0.00%> (ø)
...s/traffic_ops_golang/trafficstats/stats_summary.go 13.29% <28.76%> (+13.29%) ⬆️
traffic_ops/v5-client/stats_summary.go 82.35% <100.00%> (ø)

... and 107 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Comment thread lib/go-tc/stats_summary.go Outdated
Comment thread lib/go-tc/stats_summary.go
@jagan-parthiban jagan-parthiban force-pushed the improve/rfc3339-stats-summary branch from ce04a0b to 313da4b Compare June 2, 2023 11:59
Comment thread CHANGELOG.md
@jagan-parthiban jagan-parthiban force-pushed the improve/rfc3339-stats-summary branch from 313da4b to 48fe3d4 Compare June 5, 2023 13:51
Comment thread lib/go-tc/stats_summary.go Outdated
if resp.StatDate != nil {
statDate, err := parseTimeV5(*resp.StatDate)
if err != nil {
return errors.New("invalid timestamp given for statDate")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you add the error detail here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed It

Comment thread lib/go-tc/stats_summary.go Outdated

ss.SummaryTime, err = parseTimeV5(resp.SummaryTime)
if err != nil {
return errors.New("invalid timestamp given for summaryTime")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same comment here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed It

Alias: (Alias)(ss),
}
if ss.StatDate != nil {
resp.StatDate = util.Ptr(ss.StatDate.Format(dateFormat))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn't this also be RFC3339?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

RFC3339 will return date and time. but the expectation is only the date. So as used in the v4 function, in v5 aswell we are using dateFormat

const dateFormat = "2006-01-02"

@jagan-parthiban jagan-parthiban force-pushed the improve/rfc3339-stats-summary branch 2 times, most recently from 58c4e99 to adb19bd Compare June 14, 2023 16:01
@srijeet0406
Copy link
Copy Markdown
Contributor

Looks like the api tests are failing fir stats_summary.

@jagan-parthiban jagan-parthiban force-pushed the improve/rfc3339-stats-summary branch from b4cf7c5 to 088ef01 Compare June 18, 2023 04:56
@jagan-parthiban
Copy link
Copy Markdown
Contributor Author

Looks like the api tests are failing fir stats_summary.

The integration and unit tests are passing. This looks like the newly added contract testing.

jagan-parthiban and others added 10 commits July 3, 2023 17:24
* Added current_time_epoch_ms and changes calculation for timestamp for ms

* assigning current_time_epoch_ms to result.Time in Handler

* updated CHANGELOG.md

* Added check for elapsedTime.

* Addressed review comments.

* Added prevResult check back

* pointer assignment for structure.

* updated cache unite test with another check.
@jagan-parthiban jagan-parthiban force-pushed the improve/rfc3339-stats-summary branch from 71dc0a3 to 200b082 Compare July 3, 2023 11:57
@srijeet0406 srijeet0406 merged commit 2190cd2 into apache:master Jul 5, 2023
@rimashah25 rimashah25 added this to the 8.0.0 milestone Jul 17, 2023
@jagan-parthiban jagan-parthiban deleted the improve/rfc3339-stats-summary branch July 27, 2023 04:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

low impact affects only a small portion of a CDN, and cannot itself break one tech debt rework due to choosing easy/limited solution Traffic Ops related to Traffic Ops

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stats_Summary in TO API uses non-RFC3339 date/time strings

5 participants