Skip to content
This repository was archived by the owner on Dec 23, 2023. It is now read-only.

Tags: Add TagContextBuilder.putPropagating().#1852

Merged
songy23 merged 2 commits intocensus-instrumentation:masterfrom
songy23:put-propagating-tag
Apr 19, 2019
Merged

Tags: Add TagContextBuilder.putPropagating().#1852
songy23 merged 2 commits intocensus-instrumentation:masterfrom
songy23:put-propagating-tag

Conversation

@songy23
Copy link
Copy Markdown
Contributor

@songy23 songy23 commented Apr 19, 2019

Follow up of #1851.

Another convenient API that allows adding propagating tags without the need to always put a tag metadata.

Copy link
Copy Markdown
Contributor

@rghetia rghetia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Copy Markdown
Contributor

@dinooliva dinooliva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few minor nits but good overall.

Comment thread CHANGELOG.md Outdated
- Allow setting custom namespace in Prometheus exporter.
- Add Cumulative (`DoubleCumulative`, `LongCumulative`, `DerivedDoubleCumulative`, `DerivedLongCumulative`) APIs.
- Add a convenient API `TagContextBuilder.putLocal()` that adds non-propagating tags.
- Add convenient APIs `TagContextBuilder.putLocal()` that adds non-propagating tags,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider: convenient -> convenience

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SG, fixed.

* <p>This is equivalent to calling {@code put(key, value,
* TagMetadata.create(TagTtl.METADATA_UNLIMITED_PROPAGATION))}.
*
* <p>Only call this method if you want propagating tags. If you just want tags for breaking down
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: remove "just"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@songy23 songy23 merged commit fbd628f into census-instrumentation:master Apr 19, 2019
@songy23 songy23 deleted the put-propagating-tag branch April 19, 2019 18:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants