Added ECS Resource detector #1360
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
willarmiros
requested review from
anuraaga,
james-bebbington and
jrcamp
as code owners
anuraaga
reviewed
Oct 23, 2020
| // Parses the AWS Account ID and AWS Region from a task ARN | ||
| // See: https://docs.aws.amazon.com/AmazonECS/latest/developerguide/ecs-account-settings.html#ecs-resource-ids | ||
| func parseRegionAndAccount(taskARN string) (region string, account string) { | ||
| parts := strings.Split(taskARN, ":") |
There was a problem hiding this comment.
Not sure how important counting the colons is to follow the above advice here too
There was a problem hiding this comment.
Not really sure how to do this without making like 3 calls to LastIndexByte. I need to parse out the account_id and region from the Task ARN documented here, so counting colons matters I think.
added 2 commits
October 23, 2020 10:59
willarmiros
force-pushed
the
ecs-resources
branch
from
fe53cb8
to
a11e54e
Compare
dyladan
referenced
this pull request
in dynatrace-oss-contrib/opentelemetry-collector-contrib
Jan 29, 2021
…1360) We were not able to release yesterday because the release CI job failed. It failed because two sibling jobs persisted the same file(s) (bin/*) to the workspace. - This commit fixes the issue by storing the packaged collector in `dist/` instead of `bin/`. - In addition to that, the commit also adds job to catch any issues that might make the publish job fail two weeks down the line. It doesn't add any significant overhead. It just makes sure the job can run (no conflicting persisted files in workspace) and that the files expected by the real publish job are generated.
6 tasks
ljmsc
referenced
this pull request
in ljmsc/opentelemetry-collector-contrib
Feb 21, 2022
* Store span data directly in the span
- Nesting only some of a span's data in a `data` field (with the rest
of the data living direclty in the `span` struct) is confusing.
- export.SpanData is meant to be an immutable *snapshot* of a span,
not the "authoritative" state of the span.
- Refactor attributesMap.toSpanData into toKeyValue and make it
return a []label.KeyValue which is clearer than modifying a struct
passed to the function.
- Read droppedCount from the attributesMap as a separate operation
instead of setting it from within attributesMap.toSpanData.
- Set a span's end time in the span itself rather than in the
SpanData to allow reading the span's end time after a span has
ended.
- Set a span's end time as soon as possible within span.End so that
we don't influence the span's end time with operations such as
fetching span processors and generating span data.
- Remove error handling for uninitialized spans. This check seems to
be necessary only because we used to have an *export.SpanData field
which could be nil. Now that we no longer have this field I think we
can safely remove the check. The error isn't used anywhere else so
remove it, too.
* Store parent as trace.SpanContext
The spec requires that the parent field of a Span be a Span, a
SpanContext or null.
Rather than extracting the parent's span ID from the trace.SpanContext
which we get from the tracer, store the trace.SpanContext as is and
explicitly extract the parent's span ID where necessary.
* Add ReadOnlySpan interface
Use this interface instead of export.SpanData in places where reading
information from a span is necessary. Use export.SpanData only when
exporting spans.
* Add ReadWriteSpan interface
Use this interface instead of export.SpanData in places where it is
necessary to read information from a span and write to it at the same
time.
* Rename export.SpanData to SpanSnapshot
SpanSnapshot represents the nature of this type as well as its
intended use more accurately.
Clarify the purpose of SpanSnapshot in the docs and emphasize what
should and should not be done with it.
* Rephrase attributesMap doc comment
"refreshes" is wrong for plural ("updates").
* Refactor span.End()
- Improve accuracy of span duration. Record span end time ASAP. We
want to measure a user operation as accurately as possible, which
means we want to mark the end time of a span as soon as possible
after span.End() is called. Any operations we do inside span.End()
before storing the end time affect the total duration of the span,
and although these operations are rather fast at the moment they
still seem to affect the duration of the span by "artificially"
adding time between the start and end timestamps. This is relevant
only in cases where the end time isn't explicitly specified.
- Remove redundant idempotence check. Now that IsRecording() is based
on the value of span.endTime, IsRecording() will always return
false after span.End() had been called because span.endTime won't
be zero. This means we no longer need span.endOnce.
- Improve TestEndSpanTwice so that it also ensures subsequent calls
to span.End() don't modify the span's end time.
* Update changelog
Co-authored-by: Tyler Yahn <codingalias@gmail.com>
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
Added a new module that uses the Task Metadata Endpoint (TMDE) to record metadata for users of Otel on ECS environments. The detector is compatible with TMDE v3 and v4 on both EC2 and Fargate launch types.
This was also, AFAICT, the first processor that added an array-type attribute, and there was no way to represent such attributes in the logging exporter or in
resourcedetection.go. I submitted open-telemetry/opentelemetry-collector#1994 for the former, and did some refactoring to handle the latter in this PR.As noted in the TODOs in this PR, I'm recording some attribute values that are not part of the official spec yet. Once open-telemetry/opentelemetry-specification#1099 and open-telemetry/opentelemetry-specification#1112 are merged, I'll submit a separate PR to add them to
conventions.goin the upstream collector and replace the hardcoded values here with references to those.Link to tracking Issue: Fixes #1355
Testing: Added unit tests to the new ECS module and expanded existing test for array serialization change. I'm pretty new to unit testing in Golang, so very open to feedback on the style of the tests. Also tested with a sample app running on ECS.
Documentation: Updated resource detection
README.mdandtestdatato be in line with other detectors.