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

Segment split #102

Closed
wants to merge 15 commits into from
Closed

Segment split #102

wants to merge 15 commits into from

Conversation

atshaw43
Copy link

Description:
We are adjusting the segment creation to accommodate local root spans.

If a span is a not a local root, then we keep existing behavior.
If it is a local root then:

  1. If it is an Internal or Server span, then promote it to a segment.
  2. Else we will split it into a segment and subsegment. The segment will represent the service operation and the subsegment will represent the dependency (service A calls service B).

Testing:
Unit Testing

Documentation:
None

@atshaw43 atshaw43 marked this pull request as draft September 27, 2023 18:12
atshaw43 and others added 12 commits September 27, 2023 12:46
@atshaw43
Adding change log
cc1ce17
@atshaw43
Fixing linter issues
8d3ae2a
@djaglowski
[pkg/stanza] Fix issue where syslog octet parsing could truncate token (
d9aba7b 
open-telemetry#27294)

This fixes a bug detected while attempting to migrate test to the new
`splittest` framework.

Generally speaking, the responsibility of a `bufio.SplitFunc` is to
parse a token from a given buffer (`[]byte`). However, the split func
does not have control over the size of the buffer, so it must be able to
ask for more data. The mechanism for asking for more data is to return
`0, nil, nil`.

A split func is also told whether there is any more data to read. This
allows it to chose whether to "give up" and return a truncated token, or
to insist that it will wait until there is more data (which may never
happen).

This particular function is parsing tokens based on a simple numerical
prefix which indicates how long the token will be.
e.g. `54 This is the actual token and it is 54 characters long.`

The problem is that the function would give up prematurely and return a
truncated token. The proper behavior is to ask for more data _unless_
the function is specifically told that there is no more data to receive.

This fixes the behavior so that whenever we are able to parse an
expected length but find there is not enough data in the buffer to
fulfill the expectation, we ask for more data. It only returns a
truncated token when there is no more data to ask for.
@djaglowski
[chore][pkg/stanza] Migrate remaining split func tests to new splitte…
6aee16d 
…st framework (open-telemetry#27275)
@djaglowski
[chore][pkg/stanza] Remove defunct splittest framework (open-telemetr…
5cf472b 
…y#27296)

Follows open-telemetry#27275

All tests have been migrated away from the old `splittest` framework in
favor of the newer more thorough test. This removes the old version and
renames the file containing the new one.
@djaglowski
Revert "[tiny][pkg/stanza] Name Windows event provider on error msg" (o…
28bce8a 
…pen-telemetry#27397)

Reverts open-telemetry#27286

This is failing tests on main. I apologize for not enabling Windows
tests on the PR. @pjanotti, would you mind updating tests and
resubmitting?
@pjanotti
Restore open-telemetry#27286 and add missing corresponding test update (
9f95743 
open-telemetry#27399)

**Description:**
Restore open-telemetry#27286 (see PR open-telemetry#27397) and add missing test update.

**Link to tracking Issue:**
N/A

**Testing:**
Ran test locally.

**Documentation:**
N/A

cc @djaglowski @songy23
@atoulme
[exporter/parquet] Deprecate Parquet Exporter (open-telemetry#27290)
949f3e6 
**Description:**
Deprecate Parquet exporter before removing it altogether.

**Link to tracking Issue:**
open-telemetry#27285
@andrzej-stencel
[extension/filestorage] replace path-unsafe characters in component n…
91e56e4 
…ames (open-telemetry#20896)

Fixes open-telemetry#3148

This change was initially created to only fix open-telemetry#20731 by replacing the
slash `/` characters in component names with a tilde `~`. After that, we
decided that it is a breaking change, and so it's better to fix other
characters as well in a single breaking change.
@djaglowski
[chore][pkg/stanza] Extract max length trimming responsibility into t…
4d153b1 
…rim package (open-telemetry#27260)

This extracts the "max length" responsibility from the scanner. A couple
benefits of this:
1. The function can be applied _after_ the flush function. Previously,
if a token was flushed due to timing, we were not enforcing the max
length.
2. It makes the function directly testable.
4. We can simplify the scanner constructor, which previously had a
parameter for the buffer size which was only used for testing the
behavior of the extracted function. This is now tested within the trim
function.
@opentelemetrybot @songy23
[chore] dependabot updates Mon Oct 2 23:48:06 UTC 2023 (open-telemetr…
b5e9e5e 
…y#27400)

Bump actions/checkout from 3 to 4
Bump github.com/aws/aws-sdk-go from 1.45.17 to 1.45.19 in
/exporter/awscloudwatchlogsexporter
Bump github.com/aws/aws-sdk-go from 1.45.17 to 1.45.19 in
/exporter/awsemfexporter
Bump github.com/aws/aws-sdk-go from 1.45.17 to 1.45.19 in
/exporter/awsxrayexporter
Bump github.com/aws/aws-sdk-go from 1.45.17 to 1.45.19 in
/exporter/datadogexporter
Bump github.com/aws/aws-sdk-go from 1.45.17 to 1.45.19 in
/exporter/kafkaexporter
Bump github.com/aws/aws-sdk-go from 1.45.17 to 1.45.19 in
/internal/aws/proxy
Bump github.com/aws/aws-sdk-go from 1.45.17 to 1.45.19 in
/internal/aws/xray
Bump github.com/aws/aws-sdk-go from 1.45.17 to 1.45.19 in
/internal/aws/xray/testdata/sampleapp
Bump github.com/aws/aws-sdk-go from 1.45.17 to 1.45.19 in
/internal/metadataproviders
Bump github.com/aws/aws-sdk-go from 1.45.17 to 1.45.19 in
/processor/resourcedetectionprocessor
Bump github.com/aws/aws-sdk-go from 1.45.17 to 1.45.19 in
/receiver/awscontainerinsightreceiver
Bump github.com/aws/aws-sdk-go from 1.45.17 to 1.45.19 in
/receiver/awsecscontainermetricsreceiver
Bump github.com/aws/aws-sdk-go from 1.45.17 to 1.45.19 in
/receiver/awsxrayreceiver
Bump github.com/aws/aws-sdk-go from 1.45.17 to 1.45.20 in
/extension/observer/ecsobserver
Bump github.com/aws/aws-sdk-go from 1.45.17 to 1.45.20 in
/internal/aws/awsutil
Bump github.com/aws/aws-sdk-go from 1.45.17 to 1.45.20 in
/internal/aws/cwlogs
Bump github.com/aws/aws-sdk-go from 1.45.17 to 1.45.20 in
/internal/aws/k8s
Bump github.com/aws/aws-sdk-go-v2/config from 1.18.42 to 1.18.43 in
/exporter/awskinesisexporter
Bump github.com/aws/aws-sdk-go-v2/config from 1.18.42 to 1.18.43 in
/extension/sigv4authextension
Bump github.com/aws/aws-sdk-go-v2/credentials from 1.13.40 to 1.13.41 in
/exporter/awskinesisexporter
Bump github.com/aws/aws-sdk-go-v2/credentials from 1.13.40 to 1.13.41 in
/extension/sigv4authextension
Bump github.com/aws/aws-sdk-go-v2/service/sts from 1.22.0 to 1.23.0 in
/exporter/awskinesisexporter
Bump github.com/aws/aws-sdk-go-v2/service/sts from 1.22.0 to 1.23.0 in
/extension/sigv4authextension
Bump github.com/aws/aws-xray-sdk-go from 1.8.1 to 1.8.2 in
/internal/aws/xray/testdata/sampleapp
Bump github.com/aws/aws-xray-sdk-go from 1.8.1 to 1.8.2 in
/internal/aws/xray/testdata/sampleserver
Bump github.com/jcchavezs/porto from 0.4.0 to 0.5.1 in /internal/tools
Bump github.com/rs/cors from 1.10.0 to 1.10.1 in
/receiver/opencensusreceiver
Bump github.com/shirou/gopsutil/v3 from 3.23.8 to 3.23.9 in
/exporter/signalfxexporter
Bump github.com/shirou/gopsutil/v3 from 3.23.8 to 3.23.9 in
/extension/observer/hostobserver
Bump github.com/shirou/gopsutil/v3 from 3.23.8 to 3.23.9 in
/processor/resourcedetectionprocessor
Bump github.com/shirou/gopsutil/v3 from 3.23.8 to 3.23.9 in
/receiver/awscontainerinsightreceiver
Bump github.com/shirou/gopsutil/v3 from 3.23.8 to 3.23.9 in
/receiver/hostmetricsreceiver
Bump github.com/shirou/gopsutil/v3 from 3.23.8 to 3.23.9 in
/receiver/jmxreceiver
Bump github.com/shirou/gopsutil/v3 from 3.23.8 to 3.23.9 in /testbed
Bump github.com/snowflakedb/gosnowflake from 1.6.24 to 1.6.25 in
/receiver/sqlqueryreceiver
Bump github.com/tencentcloud/tencentcloud-sdk-go/tencentcloud/common
from 1.0.756 to 1.0.759 in /exporter/tencentcloudlogserviceexporter
Bump github.com/testcontainers/testcontainers-go from 0.23.0 to 0.25.0
in /extension/observer/dockerobserver
Bump github.com/testcontainers/testcontainers-go from 0.23.0 to 0.25.0
in /internal/coreinternal
Bump github.com/testcontainers/testcontainers-go from 0.23.0 to 0.25.0
in /receiver/aerospikereceiver
Bump github.com/testcontainers/testcontainers-go from 0.23.0 to 0.25.0
in /receiver/apachereceiver
Bump github.com/testcontainers/testcontainers-go from 0.23.0 to 0.25.0
in /receiver/dockerstatsreceiver
Bump github.com/testcontainers/testcontainers-go from 0.23.0 to 0.25.0
in /receiver/elasticsearchreceiver
Bump github.com/testcontainers/testcontainers-go from 0.23.0 to 0.25.0
in /receiver/flinkmetricsreceiver
Bump github.com/testcontainers/testcontainers-go from 0.23.0 to 0.25.0
in /receiver/jmxreceiver
Bump github.com/testcontainers/testcontainers-go from 0.23.0 to 0.25.0
in /receiver/kafkametricsreceiver
Bump github.com/testcontainers/testcontainers-go from 0.23.0 to 0.25.0
in /receiver/memcachedreceiver
Bump github.com/testcontainers/testcontainers-go from 0.23.0 to 0.25.0
in /receiver/mongodbreceiver
Bump github.com/testcontainers/testcontainers-go from 0.23.0 to 0.25.0
in /receiver/mysqlreceiver
Bump github.com/testcontainers/testcontainers-go from 0.23.0 to 0.25.0
in /receiver/nginxreceiver
Bump github.com/testcontainers/testcontainers-go from 0.23.0 to 0.25.0
in /receiver/postgresqlreceiver
Bump github.com/testcontainers/testcontainers-go from 0.23.0 to 0.25.0
in /receiver/redisreceiver
Bump github.com/testcontainers/testcontainers-go from 0.23.0 to 0.25.0
in /receiver/sqlqueryreceiver
Bump github.com/testcontainers/testcontainers-go from 0.23.0 to 0.25.0
in /receiver/zookeeperreceiver
Bump github.com/vmware/govmomi from 0.31.0 to 0.32.0 in
/receiver/vcenterreceiver
Bump
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc
from 0.44.0 to 0.45.0 in /receiver/opencensusreceiver
Bump go.opentelemetry.io/otel from 1.18.0 to 1.19.0 in /cmd/telemetrygen
Bump go.opentelemetry.io/otel from 1.18.0 to 1.19.0 in
/examples/demo/client
Bump go.opentelemetry.io/otel from 1.18.0 to 1.19.0 in
/internal/aws/metrics
Bump go.opentelemetry.io/otel from 1.18.0 to 1.19.0 in
/internal/coreinternal
Bump go.opentelemetry.io/otel from 1.18.0 to 1.19.0 in
/internal/metadataproviders
Bump go.opentelemetry.io/otel from 1.18.0 to 1.19.0 in
/processor/routingprocessor
Bump go.opentelemetry.io/otel from 1.18.0 to 1.19.0 in
/receiver/opencensusreceiver
Bump go.opentelemetry.io/otel from 1.18.0 to 1.19.0 in
/receiver/simpleprometheusreceiver/examples/federation/prom-counter
Bump go.opentelemetry.io/otel from 1.18.0 to 1.19.0 in
/receiver/solacereceiver
Bump go.opentelemetry.io/otel from 1.18.0 to 1.19.0 in
/receiver/statsdreceiver
Bump go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc
from 0.41.0 to 0.42.0 in /examples/demo/client
Bump go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc
from 0.41.0 to 0.42.0 in /examples/demo/server
Bump go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp
from 0.41.0 to 0.42.0 in /cmd/telemetrygen
Bump go.opentelemetry.io/otel/exporters/otlp/otlptrace from 1.18.0 to
1.19.0 in /cmd/telemetrygen
Bump go.opentelemetry.io/otel/exporters/otlp/otlptrace from 1.18.0 to
1.19.0 in /examples/demo/client
Bump go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc
from 1.18.0 to 1.19.0 in /examples/demo/server
Bump go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp
from 1.18.0 to 1.19.0 in /cmd/telemetrygen
Bump go.opentelemetry.io/otel/exporters/prometheus from 0.41.0 to 0.42.0
in /receiver/simpleprometheusreceiver/examples/federation/prom-counter
Bump go.opentelemetry.io/otel/metric from 1.18.0 to 1.19.0 in
/examples/demo/server
Bump go.opentelemetry.io/otel/metric from 1.18.0 to 1.19.0 in
/processor/routingprocessor
Bump go.opentelemetry.io/otel/metric from 1.18.0 to 1.19.0 in
/receiver/simpleprometheusreceiver/examples/federation/prom-counter
Bump go.opentelemetry.io/otel/schema from 0.0.4 to 0.0.7 in
/processor/schemaprocessor
Bump go.opentelemetry.io/otel/sdk from 1.18.0 to 1.19.0 in
/cmd/telemetrygen
Bump go.opentelemetry.io/otel/sdk from 1.18.0 to 1.19.0 in
/examples/demo/client
Bump go.opentelemetry.io/otel/sdk from 1.18.0 to 1.19.0 in
/internal/metadataproviders
Bump go.opentelemetry.io/otel/sdk from 1.18.0 to 1.19.0 in
/receiver/carbonreceiver
Bump go.opentelemetry.io/otel/sdk from 1.18.0 to 1.19.0 in
/receiver/opencensusreceiver
Bump go.opentelemetry.io/otel/sdk/metric from 0.41.0 to 1.19.0 in
/cmd/telemetrygen
Bump go.opentelemetry.io/otel/sdk/metric from 0.41.0 to 1.19.0 in
/examples/demo/client
Bump go.opentelemetry.io/otel/sdk/metric from 0.41.0 to 1.19.0 in
/examples/demo/server
Bump go.opentelemetry.io/otel/sdk/metric from 0.41.0 to 1.19.0 in
/receiver/simpleprometheusreceiver/examples/federation/prom-counter
Bump go.opentelemetry.io/otel/trace from 1.18.0 to 1.19.0 in
/examples/demo/server
Bump go.opentelemetry.io/otel/trace from 1.18.0 to 1.19.0 in
/internal/coreinternal
Bump go.opentelemetry.io/otel/trace from 1.18.0 to 1.19.0 in /pkg/ottl
Bump go.opentelemetry.io/otel/trace from 1.18.0 to 1.19.0 in
/processor/routingprocessor
Bump go.opentelemetry.io/otel/trace from 1.18.0 to 1.19.0 in
/processor/tailsamplingprocessor
Bump go.opentelemetry.io/otel/trace from 1.18.0 to 1.19.0 in
/receiver/opencensusreceiver

---------

Co-authored-by: Yang Song <[email protected]>
@atshaw43
Merge branch 'open-telemetry:main' into SegmentSplit
1f63ab2
@atshaw43 atshaw43 closed this Oct 3, 2023
@atshaw43 atshaw43 deleted the SegmentSplit branch October 3, 2023 17:01
thpierce
thpierce reviewed Oct 3, 2023
View reviewed changes
exporter/awsxrayexporter/internal/translator/segment.go
Comment on lines +167 to +170
// Set the name
if myAwsLocalService, ok := span.Attributes().Get(awsLocalService); ok {
serviceSegment.Name = awsxray.String(myAwsLocalService.Str())
}
Copy link

Choose a reason for hiding this comment

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

I am not sure this worked for me - I was getting the Service node named after the original SpanName when I was experimenting with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thpierce thpierce thpierce left review comments

@wangzlei wangzlei Awaiting requested review from wangzlei wangzlei will be requested when the pull request is marked ready for review wangzlei is a code owner

@mxiamxia mxiamxia Awaiting requested review from mxiamxia mxiamxia will be requested when the pull request is marked ready for review mxiamxia is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@atshaw43 @thpierce @djaglowski @pjanotti @atoulme @andrzej-stencel @opentelemetrybot