Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance telemetry API #2490

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

asankov
Copy link
Member

@asankov asankov commented Aug 18, 2021

This API will going to be used for the Performance telemetry page.
It outputs result with format:

{
  "1": {
    "1": 1
  },
  "2": {
    "1": 1
  },
  "3": {
    "1": 1
  },
  "4": {
    "3": 1
  },
  "5": {
    "null": 1
  }
}

the top-level keys are run IDs. each run ID has associated map of assignee ID - count.
this result means that for run with ID 2 there is 1 test execution, assigned to use with ID 1.

null for assignee ID means that no one is assigned for this run. we may or may not want to filter out this data.

@asankov asankov requested a review from atodorov August 18, 2021 17:00
@asankov asankov self-assigned this Aug 18, 2021
$('#id_before').on('dp.change', reloadCharts)
document.getElementById('id_test_plan').onchange = reloadCharts
document.getElementById('id_product').onchange = () => {
updateTestPlanSelectFromProduct(reloadCharts)
Copy link
Member

Choose a reason for hiding this comment

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

When changing the product I get error

Internal error: Cannot resolve keyword 'category' into field. Choices are: assignee, assignee_id, bugs, build, build_id, case, case_id, case_text_version, id, linkreference, run, run_id, sortkey, start_date, status, status_id, stop_date, tested_by, tested_by_id

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed.

'black',
'lightBlue',
'lightGreen'
]
Copy link
Member

Choose a reason for hiding this comment

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

This should be managed automatically by the charting library (d3 IIRC), no ?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes. removed

const groupedCategories = []
// map of user ID -> table column. we use map here for faster lookup by user ID.
const groupedColumnsDataMap = {}
const userIds = new Set()
Copy link
Member

Choose a reason for hiding this comment

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

You need user names. "User 5" doesn't mean anything.

Copy link
Member Author

Choose a reason for hiding this comment

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

added.

assignee_count[assignee] = 1

if execution["run_id"] != run_id:
result[run_id] = assignee_count
Copy link
Member

Choose a reason for hiding this comment

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

This can either be "TR-123" directly here or adjust the JS code to render the coordinates on the X axis as TR-123.

Copy link
Member Author

Choose a reason for hiding this comment

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

done. I changed it on the UI level.

console.log(result)

// the actual result is in the same format, only it can be much bigger
// and the chart may break
Copy link
Member

Choose a reason for hiding this comment

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

We need to have an understanding of why & how this chart breaks. If you are seeing breakage locally in devel, that's not a terribly large dataset.

Both Status matrix & Execution trends pages seem to have a larger dataset, at least in production and they seem to load just fine. On tcms.kiwitcms.org Execution trends takes a few seconds but it contains info from 5564 executions across around 1000 test runs grouped by statuses.

Copy link
Member Author

Choose a reason for hiding this comment

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

I meant this comment for here - #2490 (comment)

# count only execution which are finished
.exclude(status__weight=0)
.order_by("run_id")
.values("assignee_id", "run_id")
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't an aggregate query with Count and group by be a better choice here?

And if there isn't a big difference you can also add assignee__username into the mix.

Copy link
Member Author

Choose a reason for hiding this comment

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

the charts looks like this in my setup
image

this is for 349 executions. I am not able to find how to tell C3 to render some small part of the chart normally and add a horizontal scroll that leads to the other part. moreover, I found this issue that asks for something similar, which is still open - c3js/c3#1702 there is a workaround in the issue, I will try to do something will that

Copy link
Member Author

Choose a reason for hiding this comment

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

Wouldn't an aggregate query with Count and group by be a better choice here?

can you clarify more? what should we count by?

still WIP, because the page breaks when given too much data.
the algorithm for the data looks OK, because it works with the small
data-set I left in the code. however, given dataset with more than 500
runs it fails to display the chart miserably.
- API error, because of wrong key
- remove colors, because that is handled by C3
- add usernames, not user IDs
- add 'TR-' notation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants