#769 [frontend] Addad actions into status/stats graphs
Closed 4 years ago by praiskup. Opened 4 years ago by hojang.
copr/ hojang/copr stats  into  master

@@ -5,6 +5,7 @@ 

  from coprs import db

  from coprs import models

  from coprs import helpers

+ from coprs.logic.stat_logic import CounterStatLogic

  

  

  class ActionsLogic(object):
@@ -31,6 +32,27 @@ 

          return query

  

      @classmethod

+     def get_actions_graph_data(cls, type):

+         params = CounterStatLogic.get_graph_parameters(type)

+         results = models.Action.query\

+                 .filter(models.Action.created_on >= params["start"])\

+                 .filter(models.Action.created_on <= params["end"]) \

+                 .order_by(models.Action.created_on) \

+                 .all()

+ 

+         data = [0] * params["steps"]

+ 

+         for action in results:

+             index = 0

+             for i in range(params["start"], params["end"], params["step"]):

+                 if i < action.created_on < i + params["step"] - 1:

+                     data[index] += 1

+                 index += 1

+ 

+         data.insert(0, "actions")

+         return data

+ 

+     @classmethod

      def get_waiting(cls):

          """

          Return actions that aren't finished

@@ -30,6 +30,7 @@ 

  from coprs.models import BuildChroot

  from .coprs_logic import MockChrootsLogic

  from coprs.logic.packages_logic import PackagesLogic

+ from coprs.logic.stat_logic import CounterStatLogic

  

  log = app.logger

  
@@ -169,7 +170,7 @@ 

      @classmethod

      def get_task_graph_data(cls, type):

          data = [["pending"], ["running"], ["avg running"], ["time"]]

-         params = cls.get_graph_parameters(type)

+         params = CounterStatLogic.get_graph_parameters(type)

          cached_data = cls.get_cached_graph_data(params)

          data[0].extend(cached_data["pending"])

          data[1].extend(cached_data["running"])
@@ -197,7 +198,7 @@ 

      @classmethod

      def get_small_graph_data(cls, type):

          data = [[""]]

-         params = cls.get_graph_parameters(type)

+         params = CounterStatLogic.get_graph_parameters(type)

          cached_data = cls.get_cached_graph_data(params)

          data[0].extend(cached_data["running"])

  
@@ -231,33 +232,6 @@ 

              db.session.rollback()

  

      @classmethod

-     def get_graph_parameters(cls, type):

-         if type is "10min":

-             # 24 hours with 10 minute intervals

-             step = 600

-             steps = 144

-         elif type is "30min":

-             # 24 hours with 30 minute intervals

-             step = 1800

-             steps = 48

-         elif type is "24h":

-             # 90 days with 24 hour intervals

-             step = 86400

-             steps = 90

- 

-         end = int(time.time())

-         end = end - (end % step) # align graph interval to a multiple of step

-         start = end - (steps * step)

- 

-         return {

-             "type": type,

-             "step": step,

-             "steps": steps,

-             "start": start,

-             "end": end,

-         }

- 

-     @classmethod

      def get_build_importing_queue(cls, background=None):

          """

          Returns Builds which are waiting to be uploaded to dist git

@@ -9,6 +9,7 @@ 

  from coprs.helpers import REPO_DL_STAT_FMT, CHROOT_REPO_MD_DL_STAT_FMT, \

      CHROOT_RPMS_DL_STAT_FMT, PROJECT_RPMS_DL_STAT_FMT

  from coprs.rmodels import TimedStatEvents

+ from time import time

  

  

  class CounterStatLogic(object):
@@ -51,6 +52,33 @@ 

          return csl

  

      @classmethod

+     def get_graph_parameters(cls, type):

+         if type is "10min":

+             # 24 hours with 10 minute intervals

+             step = 600

+             steps = 144

+         elif type is "30min":

+             # 24 hours with 30 minute intervals

+             step = 1800

+             steps = 48

+         elif type is "24h":

+             # 90 days with 24 hour intervals

+             step = 86400

+             steps = 90

+ 

+         end = int(time())

+         end = end - (end % step)  # align graph interval to a multiple of step

+         start = end - (steps * step)

+ 

+         return {

+             "type": type,

+             "step": step,

+             "steps": steps,

+             "start": start,

+             "end": end,

+         }

+ 

+     @classmethod

      def get_copr_repo_dl_stat(cls, copr):

          # chroot -> stat_name

          chroot_by_stat_name = {}

@@ -48,7 +48,7 @@ 

          format: format

      };

      chart.bindto = bind;

-     chart.color.pattern = ['#0088ce', '#cc8844', '#cc0000'];

+     chart.color.pattern = ['#0088ce', '#cc8844', '#cc0000', '#008000'];

      chart.data.columns = data;

      if (format === '%Y-%m-%d')

  	chart.tooltip.format.title = function(d) {

@@ -16,10 +16,20 @@ 

      <h3>Tasks during last 24 hours</h3>

      <div id="chartDay" class="line-chart-pf"></div>

    </div>

+   <div style="color:#808080">

+     <p> pending - builds waiting in queue for builder machine </p>

+     <p> running - builds being processed </p>

+     <p> actions - fork/create/delete project, create gpg key, ... </p>

+   </div>

    <div class="row">

      <h3>Tasks during last 90 days</h3>

        <div id="chartNinetyDays" class="line-chart-pf"></div>

    </div>

+   <div style="color:#808080">

+     <p> pending - builds waiting in queue for builder machine </p>

+     <p> running - builds being processed </p>

+     <p> actions - fork/create/delete project, create gpg key, ... </p>

+    </div>

    <div class="row">

      <h3>Tasks divided by chroots</h3>

      <div class="col-md-1"></div>

@@ -5,6 +5,7 @@ 

  from coprs.views.status_ns import status_ns

  from coprs.logic import builds_logic

  from coprs.logic import complex_logic

+ from coprs.logic import actions_logic

  

  

  @status_ns.context_processor
@@ -46,6 +47,11 @@ 

      data_24h = builds_logic.BuildsLogic.get_task_graph_data('10min')

      data_90d = builds_logic.BuildsLogic.get_task_graph_data('24h')

  

+     actions_24h = actions_logic.ActionsLogic.get_actions_graph_data("10min")

+     actions_90d = actions_logic.ActionsLogic.get_actions_graph_data("24h")

+     data_24h.insert(3, actions_24h)

+     data_90d.insert(3, actions_90d)

+ 

      return flask.render_template("status/stats.html",

                                   data1=data_24h,

                                   data2=data_90d,

Here I add actions to graphs on /status/stats, soon I will add here a description of what are builds and actions.

soon I will add here a description of what are builds and actions

I will mark it work-in-progress until then.

Metadata Update from @frostyx:
- Pull-request tagged with: needs-work

4 years ago

I don't like the type change of the variable. At first, it is a query, then it gets changed to a list of results. Can you please use a different variable or just use .all() in the previous line? Also all() returns a list of things, so I would call it results not result.

This can be shortened to data = [0] * params["steps"]

Looking on BulidsLogic.get_cached_graph_data(...) vs BuildsLogic.get_task_graph_data(...), the first one looks much cleaner returning a dict instead of some sort of weird list with a key on the first position. I am not sure which method is used where, but we probably can't just switch to using dict here, right? :-/

I had some notes but otherwise, it looks fine to me.

The weird list from get_task_graph_data is needed for the graph to work. The first value is the name that's shown in the legend, followed by the values to be drawn in the graph.

rebased onto c521bf60192d2c00ddf2e8c2ee3296ef28d53d79

4 years ago

When I say I will add it later, I was thinking about in the next commit. I think that this commit complete.

No problem to add a new separate commit here in the same PR, I'd actually also prefer to solve this within this PR.

Ok, but currently my docker does not work correctly, so legend will be later.

Since you count each action only once (in one interval), you can skip the inner for cycle and just do something likeindex = (action.created_on - params["start"]) // params["step"] + 1.

As mentioned at the meeting, here's the link to the discussion about the legend tooltip. There even is a code example on how to do that but I wasn't able to get it working.

rebased onto f469623

4 years ago

Now rebased, what do you think?

Please:
- pick different color
- are we talking about "pending actions", right? please fix the legend
- could we add "running actions" as well?

different color for what?

If you mean actions, I don't choose it, I just added some data to list for javascript and the javascript choose this color.

You can change the color in frontend/coprs_frontend/coprs/static/js/graphs.js by appending to the list chart.color.pattern in lineGraph function.

2 new commits added

  • [frontend] Added legend into /status/stats describing meaning of pending/running builds and actions
  • [frontend] Addad actions into status/stats graphs
4 years ago

Please:
- pick different color
- are we talking about "pending actions", right? please fix the legend
- could we add "running actions" as well?

No, we are talking about all actions.

No, we are talking about all actions.

But it doesn't show us info whether the action queue got stucked; you mean that the graph says "how many action came in the time period" right?

I'd prefer something which would tell us the fact the action queue went nuts...

So you want to see pending and running action instead of when the action was created?

I'd like to; especially "pending" actions is very important information for us.

The problem here is that you don't have the ended_on attribute.
coprdb=# select * from action;
id | action_type | object_type | object_id | old_value | new_value | data | result | message
| created_on | ended_on
----+-------------+-------------+-----------+-----------+-----------+------------------------------------------------------------------------------------------------------------------------------------------------------+--------+---------
+------------+----------
1 | 5 | copr | | | | {"ownername": "hojang", "projectname": "test"} | 1 |
| 1559907876 |
2 | 3 | repository | 0 | | | {"ownername": "hojang", "projectname": "test", "project_dirnames": ["test"], "chroots": ["fedora-27-x86_64", "fedora-27-i386", "fedora-27-ppc64le"]} | 2 |
| 1559907881 |
3 | 3 | repository | 0 | | | {"ownername": "hojang", "projectname": "test", "project_dirnames": ["test"], "chroots": ["fedora-27-x86_64", "fedora-27-i386", "fedora-27-ppc64le"]} | 2 |
| 1559907926 |
4 | 3 | repository | 0 | | | {"ownername": "hojang", "projectname": "test", "project_dirnames": ["test"], "chroots": ["fedora-27-x86_64", "fedora-27-i386", "fedora-27-ppc64le"]} | 2 |
| 1559907927 |
5 | 3 | repository | 0 | | | {"ownername": "hojang", "projectname": "test", "project_dirnames": ["test"], "chroots": ["fedora-27-x86_64", "fedora-27-i386", "fedora-27-ppc64le"]} | 2 |
| 1559907927 |
6 | 3 | repository | 0 | | | {"ownername": "hojang", "projectname": "test", "project_dirnames": ["test"], "chroots": ["fedora-27-x86_64", "fedora-27-i386", "fedora-27-ppc64le"]} | 2 |
| 1559907928 |
7 | 3 | repository | 0 | | | {"ownername": "hojang", "projectname": "test", "project_dirnames": ["test"], "chroots": ["fedora-27-x86_64", "fedora-27-i386", "fedora-27-ppc64le"]} | 2 |
| 1559907928 |
8 | 3 | repository | 0 | | | {"ownername": "hojang", "projectname": "test", "project_dirnames": ["test"], "chroots": ["fedora-27-x86_64", "fedora-27-i386", "fedora-27-ppc64le"]} | 2 |
| 1559907928 |
9 | 3 | repository | 0 | | | {"ownername": "hojang", "projectname": "test", "project_dirnames": ["test"], "chroots": ["fedora-27-x86_64", "fedora-27-i386", "fedora-27-ppc64le"]} | 2 |
| 1559907934 |
10 | 3 | repository | 0 | | | {"ownername": "hojang", "projectname": "test", "project_dirnames": ["test"], "chroots": ["fedora-27-x86_64", "fedora-27-i386", "fedora-27-ppc64le"]} | 2 |
| 1559907949 |
11 | 3 | repository | 0 | | | {"ownername": "hojang", "projectname": "test", "project_dirnames": ["test"], "chroots": ["fedora-27-x86_64", "fedora-27-i386", "fedora-27-ppc64le"]} | 2 |
| 1559907951 |
12 | 3 | repository | 0 | | | {"ownername": "hojang", "projectname": "test", "project_dirnames": ["test"], "chroots": ["fedora-27-x86_64", "fedora-27-i386", "fedora-27-ppc64le"]} | 2 |
| 1559907951 |
13 | 3 | repository | 0 | | | {"ownername": "hojang", "projectname": "test", "project_dirnames": ["test"], "chroots": ["fedora-27-x86_64", "fedora-27-i386", "fedora-27-ppc64le"]} | 2 |
| 1559907952 |
(13 rows)

Maybe we could add the field (or start sampling the data for graphs, which would be much more relevant anyways)? I at least consider the number of incoming actions a bit useless.
Well, that is my opinion, maybe others consider that useful?

Just quick ping, @hojang do you plan to continue with this PR?

@praiskup I fear not. I don't have time and now working for another team. I ended on searching where is code, which does not write end time of action to the database. This time I need, for pending actions.

Ok, moving this to https://pagure.io/copr/copr/issue/1103 for now. We'd be glad if you could take a look at this later, so feel free to reopen.

Pull-Request has been closed by praiskup

4 years ago