Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Naming Updates #174

Closed
bobstrecansky opened this issue Sep 15, 2020 · 12 comments · Fixed by #178, #179 or #180
Closed

Naming Updates #174

bobstrecansky opened this issue Sep 15, 2020 · 12 comments · Fixed by #178, #179 or #180

Comments

@bobstrecansky
Copy link
Collaborator

To follow the spec, we should apply naming updates that were added to the specification:

Spec updates to apply for each language:
HTTPText propagator renamed to TextMap propagator (open-telemetry/opentelemetry-specification#793)
ProbabilitySampler renamed to TraceIdRatioBased (open-telemetry/opentelemetry-specification#611)
ParentOrElse renamed to ParentBased (open-telemetry/opentelemetry-specification#610)
OTLP exporter unified configuration for Tracing/Metrics (open-telemetry/opentelemetry-specification#820)
Add Translation of InstrumentationLibraryInfo to Jaeger/Zipkin

Success criteria - renaming these parts of the codebase where applicable
prondubuisi added a commit to prondubuisi/opentelemetry-php that referenced this issue Sep 19, 2020
@prondubuisi
rename instances of httptext to textmap
f8acc4e 
fixes open-telemetry#174
@prondubuisi prondubuisi mentioned this issue Sep 19, 2020
Merged
prondubuisi added a commit to prondubuisi/opentelemetry-php that referenced this issue Sep 19, 2020
@prondubuisi
change instances of probabilitysampler to traceidratiobasedsampler
0ec1674 
fixes open-telemetry#174
@prondubuisi prondubuisi mentioned this issue Sep 19, 2020
Merged
@prondubuisi
Copy link
Contributor

Hello @bobstrecansky I did not see any instances of ParentOrElse in the codebase, any pointers on this? so I can send in a PR for ParentOrElse renamed to ParentBased (open-telemetry/opentelemetry-specification#610)

prondubuisi added a commit to prondubuisi/opentelemetry-php that referenced this issue Sep 20, 2020
@prondubuisi
change instances of probabilitysampler to traceidratiobasedsampler
1f1a1d4 
force push for CLA
fixes open-telemetry#174
@beniamin
Copy link
Contributor

beniamin commented Sep 20, 2020 •
edited
Loading

@prondubuisi what you are looking for is AlwaysParentSampler which was renamed in the specifications to ParentOrElse in the PR: open-telemetry/opentelemetry-specification#609

@prondubuisi
Copy link
Contributor

Super thanks @beniamin I appreciate your help

prondubuisi added a commit to prondubuisi/opentelemetry-php that referenced this issue Sep 21, 2020
@prondubuisi
change instances of alwaysparentsampler to parentorelse
36e983d 
fixes open-telemetry#174
@prondubuisi prondubuisi mentioned this issue Sep 21, 2020
Merged
@prondubuisi
Copy link
Contributor

@prondubuisi what you are looking for is AlwaysParentSampler which was renamed in the specifications to ParentOrElse in the PR: open-telemetry/opentelemetry-specification#609

Looks like the latest Spec for this is renaming ParentOrElse to ParentBased, I will update my PR to reflect this

@prondubuisi
Copy link
Contributor

OTLP exporter unified configuration for Tracing/Metrics (open-telemetry/opentelemetry-specification#820)
Hello @beniamin @bobstrecansky I need help with understanding how to implement this specs on the codebase

bobstrecansky pushed a commit that referenced this issue Sep 21, 2020
@prondubuisi
change instances of probabilitysampler to traceidratiobasedsampler (#179
871fefb 
)

force push for CLA
fixes #174
@prondubuisi
Copy link
Contributor

Hello@bobstrecansky are we skipping some of the tasks for this issue? as tasks From point 4 are still undone

bobstrecansky pushed a commit that referenced this issue Sep 21, 2020
@prondubuisi
rename instances of httptext to textmap (#178)
7d12dc9 
fixes #174
bobstrecansky pushed a commit that referenced this issue Sep 21, 2020
@prondubuisi
change instances of alwaysparentsampler to parentorelse (#180)
517cef6 
* change instances of alwaysparentsampler to parentorelse
fixes #174

* rename instances of parentorelse to parentbased
@bobstrecansky
Copy link
Collaborator Author

@prondubuisi - I was assuming you were still looking into them. I closed this inadvertently; my apologies. Reopening.

@bobstrecansky bobstrecansky reopened this Sep 21, 2020
@bobstrecansky
Copy link
Collaborator Author

The OTLP exporter has not yet been written. I am planning on actively looking into that this week. That can be tracked with this ticket:
#130

@prondubuisi
Copy link
Contributor

@prondubuisi - I was assuming you were still looking into them. I closed this inadvertently; my apologies. Reopening.

No problems @bobstrecansky I was just curious, maybe my PR's Magically solved every task mentioned. Thanks for reopening.

@bobstrecansky
Copy link
Collaborator Author

@prondubuisi - looks like this is almost finished; did you have any other changes to make?

@prondubuisi
Copy link
Contributor

No @bobstrecansky. I think we are done. Thanks

@bobstrecansky
Copy link
Collaborator Author

Thank you @prondubuisi !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@beniamin @bobstrecansky @prondubuisi