profiles: add abstractions to assist sample composition #7727
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7727 +/- ##
=========================================
Coverage 90.16% 90.16%
Complexity 7190 7190
=========================================
Files 814 814
Lines 21702 21702
Branches 2125 2125
=========================================
Hits 19568 19568
Misses 1467 1467
Partials 667 667 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| * <p>Note that, whilst not enforced by the API, it is required that all observations in a | ||
| * collection share the same 'shape'. That is, they have either a value without timestamp, a | ||
| * timestamp without value, or both timestamp and value. Thus each array (values, timestamps) in | ||
| * the collection is either zero length, or the same length as the other. |
There was a problem hiding this comment.
this should be simple enough to enforce in this method, right? Would it be worth doing, and rejecting observations that aren't conformant?
There was a problem hiding this comment.
| public SampleCompositionKey(int stackIndex, List<Integer> attributeIndices, int linkIndex) { | ||
| this.stackIndex = stackIndex; | ||
| List<Integer> tmp = new ArrayList<>(attributeIndices); | ||
| Collections.sort(tmp); |
There was a problem hiding this comment.
Are there any restrictions on duplicate values in the indices here?
There was a problem hiding this comment.
At the spec level, no. Realistically the user level API will probably be Attributes which has Set semantics. I expect the building flow will be passing an Attributes to the dictionary to get back the List, rather than hand crafting the list, but here, as with e.g. the dictionary not enforcing referential integrity, it is possible to construct bad pointers if you try.
It's trickier than it looks to fully validate input here, as merely checking for duplicate Integers isn't sufficient to check for duplicate keys, because the underlying dictionary entries deliberately use the whole key+value+type attribute tuple, not just the key, so you have to dereference the pointers to extract the key, which you can't do without a handle on the dictionary. Likewise the stackIndex and linkIndex can't be range checked without the dictionary.
I'm not adverse to putting more safeguards in, but the current design philosophy leans more towards flexibility for users who know what they are doing, as it's focussed on use by a small set of profiler libraries than than a wide audience of application developers that something like the metrics API has. There is some help baked in, such as the sort operation here to enforce the right equality semantics, but I'm not attempting to bullet proof it.
|
@jkwatson are there specific code changes you would like here, or shall I resolve these so it can be merged? |
Adds a builder to map raw observations to SampleData objects suitable for serialization to Sample messages