-
Notifications
You must be signed in to change notification settings - Fork 629
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.
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
Add AWS resource detectors to extension package #586
Merged
lzchen
merged 35 commits into
open-telemetry:main
from
NathanielRN:add-aws-resource-detectors-to-extension
Jul 23, 2021
Merged
Changes from 1 commit
Commits
Show all changes
35 commits
Select commit
Hold shift + click to select a range
50a280c
Add raw implementation of all aws detector UNTESTED
NathanielRN 14f450a
Update semantic conventions variables
NathanielRN 0abe18b
Requests and attribute fixes
NathanielRN 7b8cf1b
Move path of detectors to shorter path
NathanielRN 2f919b3
Fix lambda directory name
NathanielRN 36dd476
Verified lambda and beanstalk detectors
NathanielRN 7bee607
Some fixes to ecs and eks detectors
NathanielRN 2b04578
Small updates to EKS resource detector
NathanielRN a72190a
Add unit tests for all resource detectors
NathanielRN f11c96f
Add readme and documentation updates
NathanielRN cf539d2
Adds useful links to explain resource detection
NathanielRN 6a1caaa
Update changelog
NathanielRN 7a73524
Fix linter issues
NathanielRN 0583ed4
Use correct black version to lint
NathanielRN e7f39ae
Use correct isort version to lint test files
NathanielRN 49790cf
Address flake lint problems
NathanielRN 5533fa1
Better naming for file stream objects for lint
NathanielRN 5e2e67a
Remove my embrassing debug statements
NathanielRN d9762e6
Use raise_on_error to make Resource detect required
NathanielRN 549b9e2
Make all debug log statements warnings instead
NathanielRN 25a904a
Run linter on recent changes
NathanielRN 3ada952
Address several lint issues with Exception too broad
NathanielRN 4756278
Update test to match previous lint updates
NathanielRN aeb838f
Raise eks excep if k8 token fetch failed
NathanielRN 9bd36f8
README subheading needs to be longer
NathanielRN cf0efc4
Small updates to address PR comments
NathanielRN e63224e
Add root prefix to ecs container file read
NathanielRN 24e8326
Linter limits lines on exception message
NathanielRN 2e8d7fe
Add root prefix to eks container file read
NathanielRN 87863eb
Eks fixes and add helpful docstring link
NathanielRN d6ad808
FileNotFoundError and allow either empty container id or cluster name…
NathanielRN 480c112
Less redundancy with general exceptions
NathanielRN 58445b8
Update test to reflect eks with only 1 open
NathanielRN 9cd22df
Python strings need to have backslash escaped
NathanielRN 530a91b
lazy loading for log messages
NathanielRN File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Move path of detectors to shorter path
- Loading branch information
commit 7b8cf1b1c189785ab11d7065a9f20b7ad511bd85
There are no files selected for viewing
27 changes: 27 additions & 0 deletions
27
.../opentelemetry-sdk-extension-aws/src/opentelemetry/sdk/extension/aws/resource/__init__.py
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
# Copyright The OpenTelemetry Authors | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
|
||
from opentelemetry.sdk.extension.aws.resource.beanstalk import AwsBeanstalkResourceDetector | ||
from opentelemetry.sdk.extension.aws.resource.ec2 import AwsEc2ResourceDetector | ||
from opentelemetry.sdk.extension.aws.resource.ecs import AwsEcsResourceDetector | ||
from opentelemetry.sdk.extension.aws.resource.eks import AwsEksResourceDetector | ||
from opentelemetry.sdk.extension.aws.resource.lambda import AwsLambdaResourceDetector | ||
|
||
__all__ = [ | ||
NathanielRN marked this conversation as resolved.
Show resolved
Hide resolved
|
||
AwsBeanstalkResourceDetector, | ||
AwsEc2ResourceDetector, | ||
AwsEc2ResourceDetector, | ||
AwsEksResourceDetector, | ||
AwsLambdaResourceDetector, | ||
] |
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit:
opentelemetry.sdk.extension.aws.resources
plural to matchopentelemetry.sdk.resources
? Not a big deal though.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I didn't even notice that, actually I did
resource
because that is the folder it is in on the specificationsAlso in java we put it under resource
And yet JavaScript, Go and PHP all put it under a folder called
detectors/
😅I guess I'll defer to the SIG here, as GCP and other vendors will probably add their resource Detectors soon... maybe we should even move the SDK import path? 😓 Although I imagine that will be hard after 1.0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We aren't at
1.0
yet so nows the time to decide on an import path :D. But again, not a blocker.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I meant we should move
opentelemetry.sdk.resources
toopentelemetry.sdk.resource
to match the spec 🙂I actually like
.resource
because there is only 1resource:
namespace in the attributes. Even if you add multipleResourceDetector
s you're still setting attributes in 1 namespace...I'll leave it for now and if we need to we'll change it before this package goes 1.0 😄. Shouldn't be a big deal to do "both" if we need to as well later!