Improve SubSegment Naming

[therealryanbonham]

Remove Invalid Chars from URL used in name to prevent errors being logged.

[therealryanbonham]

Addresses #31

[haotianw465]

Thank you for providing the fix. The log warning is at entity level to indicate the actual name is not as same as the original intent whether it is user specified or patcher code. Having the log warning usually means a bug in user code where unsupported characters are used. But in this case it reflects an issue when the SQL Alchemy patcher is assembling the name.

IMO selectively copy the logic from entity class can be a workaround. But we cannot merge this as a real fix. The subsegment name should not contain "?" or anything after "?" at the first place, so the fix should correct the information extraction from the connection string.

[thehesiod]

btw I added this for httplib: https://github.com/aws/aws-xray-sdk-python/blob/master/aws_xray_sdk/ext/util.py#L97 may prove useful

[therealryanbonham]

@haotianw465 This uses strip_url now as well as doing the logic check from entity. I also added the segment/subsegment name to the log message about invalid chars to make debuging easier for people.

[haotianw465]

This is a duplicate to https://github.com/aws/aws-xray-sdk-python/blob/master/aws_xray_sdk/core/models/entity.py#L33. IMO when we see a warning on invalid characters we fix from its source, like the strip_url here. But we should not deliberately skip the log warning by placing sub_name = ''.join([c for c in sub_name if c in _valid_name_characters]) everywhere.

Otherwise it looks fine.

[therealryanbonham]

Without that code, we would still end up logging warnings for certain SQL URLs. For example 'échéanciers' is a valid name or a database in postgres, but would be stripped to 'chanciers' for x-ray... I left this code to prevent the warnings as the sqlalchemy plugin is setting this, so it is not in users control.. If you want it removed I will, let me know.

[haotianw465]

I'm fine with special casing the sql subsegment name. I will create a follow up issue for further write an utility module that strips unicode for patchers that might capture unicode as subsegment name.

[haotianw465]

Oh one thing I forgot is that could you add one line on CHANGELOG "unreleased" section?

[thehesiod]

would be nice if it changed échéanciers to echeanciers if that makes sense

[haotianw465]

@thehesiod it's nice to have for a utility class to convert some certain unicode characters to ASCII letters. I will make sure to include that on the follow up ISSUE.