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

Add ScopedTags. #217

Closed
wants to merge 1 commit into from
Closed

Conversation

g-easy
Copy link
Contributor

@g-easy g-easy commented Oct 11, 2018

No description provided.

@g-easy
Copy link
Contributor Author

g-easy commented Oct 11, 2018

This is just a draft of the API, the implementation isn't there yet. Please let me know what you think.

@g-easy g-easy mentioned this pull request Oct 11, 2018
10 tasks
@g-easy g-easy requested a review from helly25 October 11, 2018 03:23
namespace opencensus {
namespace tags {

// ScopedTags is an RAII object, only ever stack-allocate it. While it's in
Copy link

Choose a reason for hiding this comment

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

is a RAII... While it is in

// tag key was already in use, its value will be replaced.
class ScopedTags {
public:
explicit ScopedTags(

Choose a reason for hiding this comment

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

Looking at other languages:
https://github.com/census-instrumentation/opencensus-java/blob/master/api/src/main/java/io/opencensus/tags/TagContextBuilder.java
https://github.com/census-instrumentation/opencensus-go/blob/master/tag/map.go

It seems to me that we need some possible mutations:

  • upsert/put which adds if not exist + update if exists.
  • remove which removes a tag.

Should we support these operations? Should we also offer them to modify a TagMap (not in the context)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ScopedTags does upsert.

We don't currently have a way to delete, and TagMap's API is pretty limited (I'm working on this)

@g-easy
Copy link
Contributor Author

g-easy commented Oct 12, 2018

Opened issue #223 to track this.

@g-easy g-easy closed this Oct 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants