{"action":"created","issue":{"url":"https://api.github.com/repos/census-instrumentation/instrumentation-java/issues/323","repository_url":"https://api.github.com/repos/census-instrumentation/instrumentation-java","labels_url":"https://api.github.com/repos/census-instrumentation/instrumentation-java/issues/323/labels{/name}","comments_url":"https://api.github.com/repos/census-instrumentation/instrumentation-java/issues/323/comments","events_url":"https://api.github.com/repos/census-instrumentation/instrumentation-java/issues/323/events","html_url":"https://github.com/census-instrumentation/instrumentation-java/pull/323","id":231519431,"number":323,"title":"Refactor Tag classes.","user":{"login":"sebright","id":19807615,"avatar_url":"https://avatars3.githubusercontent.com/u/19807615?v=3","gravatar_id":"","url":"https://api.github.com/users/sebright","html_url":"https://github.com/sebright","followers_url":"https://api.github.com/users/sebright/followers","following_url":"https://api.github.com/users/sebright/following{/other_user}","gists_url":"https://api.github.com/users/sebright/gists{/gist_id}","starred_url":"https://api.github.com/users/sebright/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sebright/subscriptions","organizations_url":"https://api.github.com/users/sebright/orgs","repos_url":"https://api.github.com/users/sebright/repos","events_url":"https://api.github.com/users/sebright/events{/privacy}","received_events_url":"https://api.github.com/users/sebright/received_events","type":"User","site_admin":false},"labels":[{"id":566180716,"url":"https://api.github.com/repos/census-instrumentation/instrumentation-java/labels/API","name":"API","color":"cc11aa","default":false},{"id":566180202,"url":"https://api.github.com/repos/census-instrumentation/instrumentation-java/labels/stats","name":"stats","color":"9919ff","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2017-05-26T03:54:45Z","updated_at":"2017-05-26T04:02:09Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/census-instrumentation/instrumentation-java/pulls/323","html_url":"https://github.com/census-instrumentation/instrumentation-java/pull/323","diff_url":"https://github.com/census-instrumentation/instrumentation-java/pull/323.diff","patch_url":"https://github.com/census-instrumentation/instrumentation-java/pull/323.patch"},"body":"This PR builds on top of #276.  Currently, only the last commit is new.  I made some changes to #276 after I tried modifying the rest of the code to use the new tags package.  I realized that while the type parameter in `TagKey` is convenient for setting values in a `TagMap`, it doesn't work well for specifying or querying views.  `ViewDescriptor`s/`View`s require a list of `TagKey`s or tags, so the type needed to be something like `List<TagKey<?>>`.  I think it is better to avoid using wildcards anywhere in the API, besides method parameters.\r\n\r\nI also added a `Tag` class to represent a paired `TagKey` and value.  It can be used to specify tag values in the view classes.  `Tag` and `TagKey` both need to handle three different types of values (long, boolean, and String).  However, I realized that they may need to handle the polymorphism differently, because they are used in different parts of the API.  `TagKey` is used for setting values.  For example, it is used to set tag values in a `TagMap`.  It is also used to specify views.  In constrast, `Tag` is only used for reading values.  It is returned as part of the `View` that is the result of stats collection, and it could also be used as the element type when iterating through a `TagMap`.\r\n\r\nI had a hard time coming up with a clean, simple API for `TagKey` and `Tag`.  I couldn't think of a good way to optimize both classes' APIs for both setting and reading values, so I just tried to optimize `TagKey` for setting values and `Tag` for reading values.\r\n\r\n`TagKey` exposes three specialized key types as subclasses, because it is useful to keep the static value type information when setting values in a `TagMap`:\r\n\r\n``` java\r\nTagKeyString stringKey = TagKey.createString(\"key 1\");\r\nTagKeyLong longKey = TagKey.createLong(\"key 2\");\r\nTagMap tags =\r\n    TagMap.newBuilder()\r\n        .set(stringKey, \"value\")  // must be a String\r\n        .set(longKey, 10L)        // must be a long\r\n        .build();\r\n```\r\n\r\nThen the keys can be upcast to `TagKey` and put into a list for use in a `View`:\r\n\r\n``` java\r\nList<TagKey> keys = Arrays.asList(stringKey, longKey);\r\n```\r\n\r\n`Tag` also has three specialized classes, but it doesn't expose them as subclasses, because it is only used to return results.  The subclass relationship doesn't help with reading values, unless we encourage users to downcast the values.  Here is an example of reading tags from a `DistributionAggregation` using this API:\r\n\r\n``` java\r\nList<Tag> tags = aggregation.getTags();\r\nStringBuilder sb = new StringBuilder();\r\nfor (Tag tag : tags) {\r\n  // Do three different things with the three different types of Tag:\r\n  String message = tag.match(\r\n      strTag -> strTag.getValue().asString(),\r\n      longTag -> longTag.getKey().getName(),\r\n      booleanTag -> \"boolean\");\r\n  sb.append(message);\r\n}\r\n```\r\n\r\nLet me know if you have any ideas for simplifying this API.\r\n\r\n/cc @bogdandrutu @dinooliva @songy23 "},"comment":{"url":"https://api.github.com/repos/census-instrumentation/instrumentation-java/issues/comments/304185381","html_url":"https://github.com/census-instrumentation/instrumentation-java/pull/323#issuecomment-304185381","issue_url":"https://api.github.com/repos/census-instrumentation/instrumentation-java/issues/323","id":304185381,"user":{"login":"codecov-io","id":8655789,"avatar_url":"https://avatars3.githubusercontent.com/u/8655789?v=3","gravatar_id":"","url":"https://api.github.com/users/codecov-io","html_url":"https://github.com/codecov-io","followers_url":"https://api.github.com/users/codecov-io/followers","following_url":"https://api.github.com/users/codecov-io/following{/other_user}","gists_url":"https://api.github.com/users/codecov-io/gists{/gist_id}","starred_url":"https://api.github.com/users/codecov-io/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/codecov-io/subscriptions","organizations_url":"https://api.github.com/users/codecov-io/orgs","repos_url":"https://api.github.com/users/codecov-io/repos","events_url":"https://api.github.com/users/codecov-io/events{/privacy}","received_events_url":"https://api.github.com/users/codecov-io/received_events","type":"User","site_admin":false},"created_at":"2017-05-26T04:02:09Z","updated_at":"2017-05-26T04:02:09Z","body":"# [Codecov](https://codecov.io/gh/census-instrumentation/instrumentation-java/pull/323?src=pr&el=h1) Report\n> Merging [#323](https://codecov.io/gh/census-instrumentation/instrumentation-java/pull/323?src=pr&el=desc) into [master](https://codecov.io/gh/census-instrumentation/instrumentation-java/commit/c06e0026200d7ea82d45595249715c61da608337?src=pr&el=desc) will **increase** coverage by `0.2%`.\n> The diff coverage is `94.73%`.\n\n[![Impacted file tree graph](https://codecov.io/gh/census-instrumentation/instrumentation-java/pull/323/graphs/tree.svg?src=pr&width=650&token=8575wzdhco&height=150)](https://codecov.io/gh/census-instrumentation/instrumentation-java/pull/323?src=pr&el=tree)\n\n```diff\n@@             Coverage Diff             @@\n##             master     #323     +/-   ##\n===========================================\n+ Coverage     90.59%   90.79%   +0.2%     \n- Complexity      497      523     +26     \n===========================================\n  Files            79       83      +4     \n  Lines          1977     2053     +76     \n  Branches        151      154      +3     \n===========================================\n+ Hits           1791     1864     +73     \n- Misses          150      153      +3     \n  Partials         36       36\n```\n\n\n| [Impacted Files](https://codecov.io/gh/census-instrumentation/instrumentation-java/pull/323?src=pr&el=tree) | Coverage Δ | Complexity Δ | |\n|---|---|---|---|\n| [...c/main/java/io/opencensus/tags/TagValueString.java](https://codecov.io/gh/census-instrumentation/instrumentation-java/pull/323?src=pr&el=tree#diff-Y29yZS9zcmMvbWFpbi9qYXZhL2lvL29wZW5jZW5zdXMvdGFncy9UYWdWYWx1ZVN0cmluZy5qYXZh) | `100% <100%> (ø)` | `2 <2> (?)` | |\n| [...om/google/instrumentation/internal/StringUtil.java](https://codecov.io/gh/census-instrumentation/instrumentation-java/pull/323?src=pr&el=tree#diff-Y29yZS9zcmMvbWFpbi9qYXZhL2NvbS9nb29nbGUvaW5zdHJ1bWVudGF0aW9uL2ludGVybmFsL1N0cmluZ1V0aWwuamF2YQ==) | `94.11% <100%> (+0.36%)` | `14 <3> (+3)` | :arrow_up: |\n| [core/src/main/java/io/opencensus/tags/TagKey.java](https://codecov.io/gh/census-instrumentation/instrumentation-java/pull/323?src=pr&el=tree#diff-Y29yZS9zcmMvbWFpbi9qYXZhL2lvL29wZW5jZW5zdXMvdGFncy9UYWdLZXkuamF2YQ==) | `100% <100%> (ø)` | `4 <4> (?)` | |\n| [core/src/main/java/io/opencensus/tags/Tag.java](https://codecov.io/gh/census-instrumentation/instrumentation-java/pull/323?src=pr&el=tree#diff-Y29yZS9zcmMvbWFpbi9qYXZhL2lvL29wZW5jZW5zdXMvdGFncy9UYWcuamF2YQ==) | `91.66% <91.66%> (ø)` | `12 <12> (?)` | |\n| [core/src/main/java/io/opencensus/tags/TagMap.java](https://codecov.io/gh/census-instrumentation/instrumentation-java/pull/323?src=pr&el=tree#diff-Y29yZS9zcmMvbWFpbi9qYXZhL2lvL29wZW5jZW5zdXMvdGFncy9UYWdNYXAuamF2YQ==) | `95.65% <95.65%> (ø)` | `5 <5> (?)` | |\n| [...oogle/instrumentation/trace/TraceExporterImpl.java](https://codecov.io/gh/census-instrumentation/instrumentation-java/pull/323?src=pr&el=tree#diff-Y29yZV9pbXBsL3NyYy9tYWluL2phdmEvY29tL2dvb2dsZS9pbnN0cnVtZW50YXRpb24vdHJhY2UvVHJhY2VFeHBvcnRlckltcGwuamF2YQ==) | `92.75% <0%> (+1.44%)` | `8% <0%> (ø)` | :arrow_down: |\n\n------\n\n[Continue to review full report at Codecov](https://codecov.io/gh/census-instrumentation/instrumentation-java/pull/323?src=pr&el=continue).\n> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)\n> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`\n> Powered by [Codecov](https://codecov.io/gh/census-instrumentation/instrumentation-java/pull/323?src=pr&el=footer). Last update [c06e002...2b8879b](https://codecov.io/gh/census-instrumentation/instrumentation-java/pull/323?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).\n"}}
{"actor":{"display_login":"codecov-io"}}