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

Stats: apply restriction on TagKey and TagValue #268

Closed
mayurkale22 opened this issue Jan 9, 2019 · 5 comments
Closed

Stats: apply restriction on TagKey and TagValue #268

mayurkale22 opened this issue Jan 9, 2019 · 5 comments

Comments

@mayurkale22
Copy link
Member

The TagKey name must meet the following requirements:

  1. Must contain only printable ASCII (codes between 32 and 126 inclusive)
  2. Must have length greater than zero and less than 256.
  3. Must not be empty.

The TagValue name must meet the following requirements:

  1. It MUST contain only printable ASCII (codes between 32 and 126)

Specs: https://github.com/census-instrumentation/opencensus-specs/blob/master/tags/TagMap.md#tagkey

@vigneshtdev
Copy link
Contributor

hey i would like to help with this issue. In opencensus java it had a package called tag where different tagvalue & tagkey can be configured. Where should I look for it in this package?

@mayurkale22
Copy link
Member Author

@vigneshtdev Thanks for your interest in the project, all contributions are welcomed and highly appreciated. Currently we dont have separate package for Tags api. I think we should create new one same as Java and Python. WDYT?

@vigneshtdev
Copy link
Contributor

vigneshtdev commented Jan 12, 2019

I think that it is the correct way to go, as it will make implementation across all languages more consistent. However it would require major refactoring in both test & soruce

@vigneshtdev
Copy link
Contributor

Or else if such refactoring in unnecessary we can modify the existing invalidTags method. Currenlty it only check if the tagkey & tagvalue have printable characters.

mayurkale22 pushed a commit that referenced this issue Jan 16, 2019
* Add validation for tagkey & tagvalue. Fix issue #268

* Fix formatting errors

* Log warning to screen when invalid tag has been used

* Remove unnecessary import

* Fix formatting error

* Replace long string in test case
@songy23
Copy link
Contributor

songy23 commented Jan 17, 2019

Fixed in #280.

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

3 participants