Open basepi opened 4 years ago
Pinging @elastic/apm-ui (Team:apm)
Is this for the metadata?
Or local variables?
This would be for local variables :) thanks for asking for clarification!
Can you post an example of how this would look in elasticsearch? I tried looking at the opbeans-python generated data but didn't see it.
You should get it if you boot up a stock compose.py with --with-opbeans-python
. If you use your central instance, you might need to re-run compose.py
there to ensure that the newest opbeans versions are running. It should be in the AssertionError: Bad luck
error.
Anyway, here's an example frame I grabbed from my local instance:
{
"library_frame": false,
"exclude_from_grouping": false,
"filename": "opbeans/tasks.py",
"abs_path": "/app/opbeans/tasks.py",
"line": {
"number": 19,
"context": " assert False, \"Bad luck!\""
},
"module": "opbeans.tasks",
"function": "update_stats",
"context": {
"pre": [
" if random.random() > 0.8:",
" dict_for_truncation = {k: k for k in range(500)}"
],
"post": [
" cache.set(utils.stats.cache_key, utils.stats(), 60)",
""
]
},
"vars": {
"dict_for_truncation": {
"0": 0,
"1": 1,
"2": 2,
"3": 3,
"4": 4,
"5": 5,
"6": 6,
"7": 7,
"8": 8,
"9": 9,
"<truncated>": "(490 more elements)"
}
}
}
It is already sorted correctly here. But if one of the keys is non-ASCII, (e.g. a Chinese character), <truncated>
will not appear last in an alphabetic order.
I'm not sure the best approach is simply to sort the variables and show <truncate>
last. Another option could be to not show <truncate>
at all, and instead displaying a callout below the table saying something like
490 variables are not being displayed. Read more in the docs to configure the number of variables captured.
Sounds like we need some design expertise so pulling in @formgeist.
Btw. Is the python agent the only one using this feature? Or do we need to align this with other agent devs?
Btw. Renamed the issue to focus more on the problem we are trying to solve instead of a specific solution. LMK if my edits are inaccurate.
AFAIK, only the Python agent supports collection of local variables.
We suggested the above solution as it aligns with what we're already doing for lists. There it'll show "(490 more elements)"
as last item of the list. I don't really think a more advanced solution is needed for this, but don't let me stop you from going all in ;p
I don't really think a more advanced solution is needed for this, but don't let me stop you from going all in
In the end we might go for the solution you presented. Just want to keep the options open before deciding on something :)
Not to get all fancy, but how about (based on @sqren's suggestion) we display a message like this to make it a little more user-friendly to read (perhaps to someone who didn't implement it).
That looks nice! Two notes:
it's possible and probable that there are more variables below the container that was truncated. Will the design still work with that?
I'm not following this. If the suggestion was to write "<truncated>": "(490 more elements)"
at the bottom of the table, why wouldn't it work writing it out as a sentence below the table?
Imagine you have 3 local variables
a = "foo"
b = {i: i for i in range(100)} # a dict/map with 100 key/value pairs
c = "bar"
This would currently be rendered as such:
a "foo"
b.0 0
b.1 1
b.2 2
b.3 3
b.4 4
b.5 5
b.6 6
b.7 7
b.8 8
b.9 9
b.<truncated> (90 more elements)
c "bar"
The truncation only applies on b
, not on a
or c
.
Ahh, that's new information to me. So there could be multiple truncated
items in the variables table?
Sorry for being unclear, but that's what we're trying to say from the beginning :) we're truncating variables that have too many elements (lists, dicts, ...), not the list of variables. So yes, there can be multiple variables of which the content has been truncated.
@beniwohli Thanks for clarifying (I also needed to understand what was meant in the original statement). I've updated the design so that we'll keep the original <truncated>: (490 more elements)
by each variable. Then at the end of the table we can display a user-friendly message to help users understand how they might go about changing those configurations.
Thoughts?
Not sure if it's just because it is a mockup, but content-wise it still isn't quite right. dict_for_truncation
is one variable, a dictionary. It originally had 500 elements ({0: 0, 1: 1, ...}
), we throw away 490 of them agent-side when serializing it, and instead add a new element, "<truncated>": "(490 more elements)"
. So dict_for_truncation
will not have multiple <truncated>
elements, only one. It's possible that there is another dict (or list) that got truncated, and that one would have its own <truncated>
element, too.
There's also a small problem with providing a link, as there are multiple config options which may cause truncation (local_var_max_length
, local_list_var_max_length
and local_var_dict_max_length
).
Fair enough, my mock was probably a little too hastily put together, I should have renamed the variables. It was more to indicate that you might have multiple variables that are truncated. It would be possible to make a section in the docs that reference the different config options that could so we have a single point of reference. It can also be left out. I guess this somehow leaves us back at the original implementation (no particular UI design)?
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
Summary Ensure that
<truncated>
is last:Background In the APM Python Agent, we added a feature to truncate large dictionaries during local variable collection for errors, to ensure that we don't send too much data to the APM server.
We wanted to notify the user that this occurred, so we added a new key to the newly-shortened dictionary,
"<truncated>"
, the value of which tells the user how many keys were dropped from the dictionary.We'd like to override the default alphabetical sorting for this key in order to show it last, to make it more obvious that there used to be more data.
Keep me posted if you need more information.
cc @beniwohli