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

Cleanup manually/automatic generated class in pdata related to trace #2488

Closed
16 tasks done
bogdandrutu opened this issue Feb 13, 2021 · 15 comments
Closed
16 tasks done

Cleanup manually/automatic generated class in pdata related to trace #2488

bogdandrutu opened this issue Feb 13, 2021 · 15 comments
Assignees
@punya @Aneurysm9 @bogdandrutu
Labels
release:required-for-ga Must be resolved before GA release
Milestone
Phase1-Trace-GA-R...

Comments

@bogdandrutu
Copy link
Member

bogdandrutu commented Feb 13, 2021 •
edited
Loading

This is a preparation for pdata (TraceID, SpanID, AttributeValue, AttributeMap) to be marked as stable API.

  • Cleanup translator code to work only with pdata types (TraceID, SpanID)
  • Should we consider to move translator code as helper to pdata (NewTraceIDFromLong, NewSpanIDFromLong, etc.). Translator code is in translator/trace/big_endian_converter?
  • Review public API for TraceID, SpanID - Punya
  • Review public API for AttributeValue and AttributeMap.
    • One thing that we should consider is to rename ForEach with Range to be consistent with sync.Map.
    • Remove deprecated NewAttributeValue, needs changes in pdatagen which is the only user of this.
    • Remove InitEmptyWithCapacity
    • Add EnsureCapacity (replacement for InitEmptyWithCapacity) that does not remove existing elements.
  • Review comments for all public API, maybe we have things to improve. - Punya
  • Review the Resize api for slices.
    • MAY consider to replace the API with EnsureCapacity + AppendEmpty usage. Or AppendEmptyN(int).
  • Remove deprecated Resize - Anthony
  • Remove deprecated Append api for slices.
    • Cleanup core usages of pdata.Append.
    • Cleanup contrib usages of pdata.Append.
  • Consider to change StartTime/EndTime to StartTimestamp/EndTimestamp
@bogdandrutu bogdandrutu added this to the Phase1-GA-Roadmap milestone Feb 13, 2021
This was referenced Feb 13, 2021
Merged
Closed
Closed
Closed
Closed
This was referenced Feb 20, 2021
Merged
Merged
Merged
bogdandrutu added a commit that referenced this issue Feb 22, 2021
@bogdandrutu
Remove deprecated function IsValid from trace/span ID (#2522)
d71dd31 
Updates #2488

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
bogdandrutu added a commit to bogdandrutu/opentelemetry-collector that referenced this issue Feb 24, 2021
@bogdandrutu
Cleanup usage of TimestampUnixNanos and its API
c231b80 
Updates open-telemetry#2488

Important Changes:

* Rename pdata.TimestampUnixNanos to pdata.Timestamp
* Remove pdata.TimestampUnixNanos and helpers, move them of the pdata.Timestamp type.
* Fix bug around IsZero, this function should return true if the time is January 1, year 1, 00:00:00 UTC not epoch.

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
@bogdandrutu bogdandrutu mentioned this issue Feb 24, 2021
Merged
bogdandrutu added a commit to bogdandrutu/opentelemetry-collector that referenced this issue Feb 24, 2021
@bogdandrutu
Cleanup usage of TimestampUnixNanos and its API
f9dea87 
Updates open-telemetry#2488

Important Changes:

* Rename pdata.TimestampUnixNanos to pdata.Timestamp
* Remove pdata.TimestampUnixNanos and helpers, move them of the pdata.Timestamp type.
* Fix bug around IsZero, this function should return true if the time is January 1, year 1, 00:00:00 UTC not epoch.

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
bogdandrutu added a commit to bogdandrutu/opentelemetry-collector that referenced this issue Feb 24, 2021
@bogdandrutu
Cleanup usage of TimestampUnixNanos and its API
f80a3c6 
Updates open-telemetry#2488

Important Changes:

* Rename pdata.TimestampUnixNanos to pdata.Timestamp
* Remove pdata.TimestampUnixNanos and helpers, move them of the pdata.Timestamp type.
* Fix bug around IsZero, this function should return true if the time is January 1, year 1, 00:00:00 UTC not epoch.

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
bogdandrutu added a commit to bogdandrutu/opentelemetry-collector that referenced this issue Feb 25, 2021
@bogdandrutu
Cleanup usage of TimestampUnixNanos and its API
e9a4c2a 
Updates open-telemetry#2488

Important Changes:

* Rename pdata.TimestampUnixNanos to pdata.Timestamp
* Remove pdata.TimestampUnixNanos and helpers, move them of the pdata.Timestamp type.
* Fix bug around IsZero, this function should return true if the time is January 1, year 1, 00:00:00 UTC not epoch.

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
bogdandrutu added a commit that referenced this issue Feb 25, 2021
@bogdandrutu
Cleanup usage of TimestampUnixNanos and its API (#2549)
0a8a1dc 
Updates #2488

Important Changes:

* Rename pdata.TimestampUnixNanos to pdata.Timestamp
* Remove pdata.TimestampUnixNanos and helpers, move them of the pdata.Timestamp type.
* Fix bug around IsZero, this function should return true if the time is January 1, year 1, 00:00:00 UTC not epoch.

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
@alolita
Copy link
Member

alolita commented Mar 11, 2021

@bogdandrutu can you assign @Aneurysm9 to this issue?

This was referenced Mar 30, 2021
Merged
Merged
This was referenced Apr 15, 2021
Merged
Merged
@bogdandrutu bogdandrutu changed the title Cleanup manually generated class in pdata related to trace Cleanup manually/automatic generated class in pdata related to trace Apr 20, 2021
@alolita
Copy link
Member

alolita commented May 12, 2021

@bogdandrutu is this issue considered complete with @Aneurysm9 's PRs? Can we close this issue?

@bogdandrutu bogdandrutu mentioned this issue May 14, 2021
Closed
10 tasks
@bogdandrutu
Copy link
Member Author

@alolita I have not seen any PR from @Aneurysm9 related to this, we discussed that this is a larger effort, see issue description

@alolita alolita added the release:required-for-ga Must be resolved before GA release label May 18, 2021
@alolita
Copy link
Member

alolita commented Jun 14, 2021

@bogdandrutu we're working on this - will sync up w @Aneurysm9

@bogdandrutu
Copy link
Member Author

@alolita this is one of the blockers to finish Phase1 milestone.

@Aneurysm9 Aneurysm9 mentioned this issue Jun 22, 2021
Merged
@jpkrohling
Copy link
Member

About "eliminate use of .Append", what's the reasoning? Callers now have to make two calls to have the same effect: https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/3861/files#diff-6211e1422cb9d2f03e84e85892a9a659a0d7d3d42de41cae0d130f63767d8a56L479-R482

This was referenced Jun 29, 2021
Merged
Merged
@alolita
Copy link
Member

alolita commented Jun 30, 2021

@punya As we discussed assigning you the following reviews to help complete the tasks itemized in this issue.

  1. Review public API for TraceID, SpanID
  2. Review comments for all public API, maybe we have things to improve.

@alolita
Copy link
Member

alolita commented Jun 30, 2021

@Aneurysm9 do you have bandwidth to complete the last review and related PR for the changes. This would help us get all items in this issue done :-)

  1. Review the Resize api for slices.
    ** You MAY consider replacing the API with EnsureCapacity + AppendEmpty usage. Or AppendEmptyN(int).

@Aneurysm9
Copy link
Member

@Aneurysm9 do you have bandwidth to complete the last review and related PR for the changes. This would help us get all items in this issue done :-)

1. Review the Resize api for slices.
   ** You MAY consider replacing the API with EnsureCapacity + AppendEmpty usage. Or AppendEmptyN(int).

I will get started on this.

@bogdandrutu am I understanding correctly that we will remove the ability to resize a slice downward? EnsureCapacity will only be able to increase the capacity of a slice.

@alolita
Copy link
Member

alolita commented Jul 6, 2021

@punya will you still be able to work on these reviews?

@Aneurysm9 Aneurysm9 mentioned this issue Jul 6, 2021
Merged
@punya
Copy link
Member

punya commented Jul 13, 2021

@alolita yes (we discussed offline but I'm commenting here for the record).

@punya
Copy link
Member

punya commented Jul 13, 2021

AttributeValue.Equals() has a TODO for supporting map data types. Should this be considered a GA blocker?

This was referenced Jul 13, 2021
Merged
Merged
@alolita
Copy link
Member

alolita commented Jul 14, 2021

Action Item: Remove deprecated Resize -- @Aneurysm9 please complete the removal of usage of Resize from contrib and definitions from core.

@Aneurysm9 Aneurysm9 mentioned this issue Jul 15, 2021
Merged
@alolita
Copy link
Member

alolita commented Jul 15, 2021

Thanks @Aneurysm9

This was referenced Jul 15, 2021
Merged
Merged
tigrannajaryan pushed a commit to open-telemetry/opentelemetry-collector-contrib that referenced this issue Jul 16, 2021
@Aneurysm9
exporter/*: remove use of pdata slice Resize() (#4203)
d31c58a 
Eliminates use of deprecated pdata slice APIs in exporters
**Link to tracking Issue:** open-telemetry/opentelemetry-collector#2488

Signed-off-by: Anthony J Mirabella <a9@aneurysm9.com>
@alolita
Copy link
Member

alolita commented Jul 19, 2021

@Aneurysm9 are there PRs in flight for collector changes for removing pdata slice Resize()?

@Aneurysm9 Aneurysm9 mentioned this issue Jul 19, 2021
Merged
punya pushed a commit to punya/opentelemetry-collector-contrib that referenced this issue Jul 21, 2021
@bogdandrutu
Cleanup usage of TimestampUnixNanos and its API (open-telemetry#2549)
56d8b75 
Updates open-telemetry/opentelemetry-collector#2488

Important Changes:

* Rename pdata.TimestampUnixNanos to pdata.Timestamp
* Remove pdata.TimestampUnixNanos and helpers, move them of the pdata.Timestamp type.
* Fix bug around IsZero, this function should return true if the time is January 1, year 1, 00:00:00 UTC not epoch.

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
punya pushed a commit to punya/opentelemetry-collector-contrib that referenced this issue Jul 21, 2021
@bogdandrutu
Cleanup usage of TimestampUnixNanos and its API (open-telemetry#2549)
7686de0 
Updates open-telemetry/opentelemetry-collector#2488

Important Changes:

* Rename pdata.TimestampUnixNanos to pdata.Timestamp
* Remove pdata.TimestampUnixNanos and helpers, move them of the pdata.Timestamp type.
* Fix bug around IsZero, this function should return true if the time is January 1, year 1, 00:00:00 UTC not epoch.

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
hughesjj added a commit to hughesjj/opentelemetry-collector that referenced this issue Apr 27, 2023
@hughesjj @atoulme
Switch to gcp from gce/gke (open-telemetry#2488)
e7f0204 
* Switch to gcp from gce as specified in open-telemetry/opentelemetry-collector-contrib#10348
Tracking issue: signalfx/splunk-otel-collector#2474

* Update cmd/otelcol/config/collector/full_config_linux.yaml

Co-authored-by: Antoine Toulme <antoine@lunar-ocean.com>

Co-authored-by: Antoine Toulme <antoine@lunar-ocean.com>
Troels51 pushed a commit to Troels51/opentelemetry-collector that referenced this issue Jul 5, 2024
@marcalff
[PROTO] Upgrade to opentelemetry-proto v1.1.0 (open-telemetry#2488)
423ecac
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@punya punya

@Aneurysm9 Aneurysm9

@bogdandrutu bogdandrutu

Labels
release:required-for-ga Must be resolved before GA release
Projects
None yet
Milestone
Phase1-Trace-GA-Roadmap
Development

No branches or pull requests
5 participants
@jpkrohling @punya @Aneurysm9 @bogdandrutu @alolita