Skip to content

Fix dashboard functionalities #84

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

Merged
merged 13 commits into from
Apr 11, 2019
9 changes: 4 additions & 5 deletions examples/create_dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def usage():
usage()

# Name for the dashboard to create
dashboardName = "API test - cassandra in prod"
dashboardName = "Overview by Process"
for opt, arg in opts:
if opt in ("-d", "--dashboard"):
dashboardName = arg
Expand All @@ -55,8 +55,7 @@ def usage():
# in Sysdig Cloud Explore page.
# You can also refer to AWS tags by using "cloudProvider.tag.*" metadata or
# agent tags by using "agent.tag.*" metadata
dashboardFilter = "kubernetes.namespace.name = prod and proc.name = cassandra"

dashboardFilter = "kubernetes.namespace.name = prod"
print('Creating dashboard from view')
ok, res = sdclient.create_dashboard_from_view(dashboardName, viewName, dashboardFilter)
#
Expand All @@ -74,9 +73,9 @@ def usage():
#

# Name of the dashboard to copy
dashboardCopy = "Copy Of {}".format(dashboardName)
dashboardCopy = "Copy of {}".format(dashboardName)
# Filter to apply to the new dashboard. Same as above.
dashboardFilter = "kubernetes.namespace.name = dev and proc.name = cassandra"
dashboardFilter = "kubernetes.namespace.name != prod"

print('Creating dashboard from dashboard')
ok, res = sdclient.create_dashboard_from_dashboard(dashboardCopy, dashboardName, dashboardFilter)
Expand Down
98 changes: 69 additions & 29 deletions sdcclient/_monitor.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import json
import copy
import requests
import re

from sdcclient._common import _SdcCommon

Expand Down Expand Up @@ -430,31 +431,10 @@ def add_dashboard_panel(self, dashboard, name, panel_type, metrics, scope=None,
'groupAggregation': metric['aggregations']['group'] if 'aggregations' in metric else None,
'propertyName': property_name + str(i)
})
#
# Convert scope to format used by Sysdig Monitor
#
if scope != None:
filter_expressions = scope.strip(' \t\n\r?!.').split(" and ")
filters = []

for filter_expression in filter_expressions:
values = filter_expression.strip(' \t\n\r?!.').split("=")
if len(values) != 2:
return [False, "invalid scope format"]
filters.append({
'metric': values[0].strip(' \t\n\r?!.'),
'op': '=',
'value': values[1].strip(' \t\n\r"?!.'),
'filters': None
})

if len(filters) > 0:
panel_configuration['filter'] = {
'filters': {
'logic': 'and',
'filters': filters
}
}
panel_configuration['scope'] = scope
# if chart scope is equal to dashboard scope, set it as non override
panel_configuration['overrideFilter'] = ('scope' in dashboard and dashboard['scope'] != scope) or ('scope' not in dashboard and scope != None)

#
# Configure panel type
Expand Down Expand Up @@ -580,17 +560,31 @@ def create_dashboard_from_template(self, dashboard_name, template, scope, shared
template['isPublic'] = public
template['publicToken'] = None

#
# set dashboard scope to the specific parameter
# NOTE: Individual panels might override the dashboard scope, the override will NOT be reset
#
scopeExpression = self.convert_scope_string_to_expression(scope)
if scopeExpression[0] == False:
return scopeExpression
template['filterExpression'] = scope
template['scopeExpressionList'] = map(lambda ex: {'operand':ex['operand'], 'operator':ex['operator'],'value':ex['value'],'displayName':'', 'isVariable':False}, scopeExpression[1])

if 'items' in template:
if 'widgets' in template and template['widgets'] is not None:
# Default dashboards (aka Explore views) specify panels with the property `widgets`,
# while custom dashboards use `items`
template['items'] = list(template['widgets'])
del template['widgets']

# NOTE: Individual panels might override the dashboard scope, the override will NOT be reset
if 'items' in template and template['items'] is not None:
for chart in template['items']:
if 'overrideFilter' in chart and chart['overrideFilter'] == False:
if 'overrideFilter' not in chart:
chart['overrideFilter'] = False

if chart['overrideFilter'] == False:
# patch frontend bug to hide scope override warning even when it's not really overridden
chart['scope'] = scope

# if chart scope is equal to dashboard scope, set it as non override
chart['overrideFilter'] = chart['scope'] != scope

if 'annotations' in template:
template['annotations'].update(annotations)
Expand Down Expand Up @@ -754,6 +748,52 @@ def get_metrics(self):
res = requests.get(self.url + '/api/data/metrics', headers=self.hdrs, verify=self.ssl_verify)
return self._request_result(res)

def convert_scope_string_to_expression(self, scope):
if scope != None:
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer:

if scope is None:
    return [True, None]

In fact I would return [True, []] so we return always the same type. [] is also False when in conditions.

expressions = []
string_expressions = scope.strip(' \t\n\r').split(' and ')
expression_re = re.compile('^(?P<not>not )?(?P<operand>[^ ]+) (?P<operator>=|!=|in|contains|starts with) (?P<value>.+)$')

for string_expression in string_expressions:
matches = expression_re.match(string_expression)

if matches == None:
return [False, 'invalid scope format']

is_not = matches.group('not') != None
Copy link
Contributor

Choose a reason for hiding this comment

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

PEP8 recommendations say we should use is None and is not None instead of == None and != None. This is happening many times at the PR, so I guess you are using == to maintain code homogeneity, in that case 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are they functionally equivalent? Also, this reminds me that we should really add "linting" validation soon...


if matches.group('operator') == 'in':
list_value = matches.group('value').strip(' ()')
value_matches = re.findall('[^,]+', list_value)

if len(value_matches) == 0:
Copy link
Contributor

Choose a reason for hiding this comment

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

PEP8 says we should take the advantage that not value_matches means the same.

return [False, 'invalid scope list format']

values = map(lambda v: v.strip(' "\''), value_matches)
else:
values = [matches.group('value').strip('"\'')]

if matches.group('operator') == 'in':
operator = 'in' if is_not == False else 'notIn'
elif matches.group('operator') == '=':
operator = 'equals'
elif matches.group('operator') == '!=':
operator = 'notEquals'
elif matches.group('operator') == 'contains':
operator = 'contains' if is_not == False else 'notContains'
Copy link
Contributor

Choose a reason for hiding this comment

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

is_not == False should be changed to not is_not (honestly I would refactor is_not name because of this to something more descriptive.

This is happening more times at the PR. Again, if it is to maintain existing code style, I'm OK.

elif matches.group('operator') == 'starts with':
operator = 'startsWith'
Copy link
Contributor

Choose a reason for hiding this comment

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

Running away from if/elif we could:

operator_parse_dict = {
    'in': 'in' if not is_not else 'notIn',
    '=': 'equals',
    ....
}
operator = operator_parse_dict.get(matches.group('operator'), None)


expressions.append({
'operand': matches.group('operand'),
'operator': operator,
'value': values
})

return [True, expressions]
else:
return [True, None]


# For backwards compatibility
SdcClient = SdMonitorClient