Skip to content
This repository has been archived by the owner on Aug 4, 2023. It is now read-only.

Truncate keys #41

Closed
watson opened this issue Dec 18, 2018 · 9 comments
Closed

Truncate keys #41

watson opened this issue Dec 18, 2018 · 9 comments

Comments

@watson
Copy link
Contributor

watson commented Dec 18, 2018

Today we just truncate object values, but we should probably truncate keys with the same algorithm we use to truncate values.

@alemagio
Copy link
Contributor

alemagio commented Oct 1, 2019

Hi, i would like to work on this.
I think that i should work on the truncate.js file and use the onObject callback (where i have access to the key) instead of the onValue (which is the one truncating now).
Right?

@Qard
Copy link
Contributor

Qard commented Oct 1, 2019

It might be achievable with onObject. It could be a bit of a challenge though as it's a bit tough to wrap your brain around the nested filter stuff--you could very easily blow away your own changes or send it into an infinite loop and blow the stack. Feel free to ask questions here if you get stuck and I'll do what I can to help. 👍

@alemagio
Copy link
Contributor

alemagio commented Oct 4, 2019

Hi, i started looking for this and I'm not sure what the expectation is.
If I take as an expample the truncateSpan function you expect that, given an input span like this:
{
foo: "bar",
}
and an option that sets the max key length to 2 the output should be:
{
fo: "bar",
}
Correct?

@Qard
Copy link
Contributor

Qard commented Oct 4, 2019

Yes. Recursively, all keys should be shortened. One challenge that we never put much thought into how to handle is what to do about collisions. For example, shortening a1 and a2 to key length 1 would result in two different values for a--which one should it finally be? I'm not really sure. 🤔

@watson
Copy link
Contributor Author

watson commented Oct 7, 2019

I would consider just truncating the keys that are controlled by the user. Might be easier on the CPU

@alemagio
Copy link
Contributor

alemagio commented Oct 7, 2019

Ok, but the issue regarding the collisions remains right?

@Qard
Copy link
Contributor

Qard commented Oct 7, 2019

Yes, collisions are still possible.

alemagio added a commit to alemagio/apm-nodejs-http-client that referenced this issue Oct 12, 2019
@alemagio
Copy link
Contributor

Just created a PR
It is not complete yet but i think it could be useful to make me understand if i'm on the right way
Let me know ;)

alemagio added a commit to alemagio/apm-nodejs-http-client that referenced this issue Oct 22, 2019
@Qard
Copy link
Contributor

Qard commented Oct 23, 2019

Closed by #92.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants