-
Notifications
You must be signed in to change notification settings - Fork 224
feat: add Render resource detector #1667
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
base: main
Are you sure you want to change the base?
Conversation
955bdca to
45b2000
Compare
| if ENV.fetch('RENDER', false) == 'true' | ||
| resource_attributes[RESOURCE::CLOUD_PROVIDER] = 'render' | ||
| resource_attributes['render.is_pull_request'] = ENV.fetch('IS_PULL_REQUEST', 'false') | ||
| resource_attributes['render.git.branch'] = ENV.fetch('RENDER_GIT_BRANCH', nil) |
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.
Should be vcs.ref.head.name
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.
Thanks, will change.
| resource_attributes[RESOURCE::CLOUD_PROVIDER] = 'render' | ||
| resource_attributes['render.is_pull_request'] = ENV.fetch('IS_PULL_REQUEST', 'false') | ||
| resource_attributes['render.git.branch'] = ENV.fetch('RENDER_GIT_BRANCH', nil) | ||
| resource_attributes['render.git.repo_slug'] = ENV.fetch('RENDER_GIT_REPO_SLUG', nil) |
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.
Could we construct a vcs.repository.url.full using the slug?
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.
No. Render supports multiple git backend but does not expose this information via an environment variable.
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.
In that case I would suggest vcs.repository.slug and get the attribute defined.
| resource_attributes['render.git.repo_slug'] = ENV.fetch('RENDER_GIT_REPO_SLUG', nil) | ||
| resource_attributes[RESOURCE::SERVICE_INSTANCE_ID] = ENV.fetch('RENDER_INSTANCE_ID', nil) | ||
| resource_attributes[RESOURCE::SERVICE_NAME] = ENV.fetch('RENDER_SERVICE_NAME', 'unknown_service') | ||
| resource_attributes[RESOURCE::SERVICE_VERSION] = ENV.fetch('RENDER_GIT_COMMIT', nil) |
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.
Should this not be using a vcs.ref.head.revision attribute instead?
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.
I think both can apply, eg we deploy from main and do not have any other kind of version identifier for our app. Software like GitLab does have a version number and a revision, so I'll change this to allow for that kind of thing.
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.
For me usually the service attributes should be driven by the application and not by the build/deployment system. Maybe you could use artifact.name/version to capture these variables.
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.
I see
opentelemetry-ruby-contrib/instrumentation/rails/lib/opentelemetry/instrumentation/rails/railtie.rb
Line 20 in 44337aa
| ENV['OTEL_SERVICE_NAME'] ||= app.class.name.split('::').first.underscore |
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.
So looking at what other solutions ie heroku, gcp, cloud foundary do which is capture this info in their own namespace. Hence i would not set the service.name/version attributes via this resource detector but rather introduce with render.app.name & render.app.version to store this.
|
Thank you for this contribution. I am personally not a user of the Render platform nor part of their engineering team. I do not know much about this platform so I am a little reluctant to add it to the contrib repo. What level of commitment would you be able to provide for ongoing maintenance of this resource detector? Do you have any contacts at render that would be willing to take ownership or maintenance of this long term? |
This implements a resource detector for the Render platform. I used the AWS EC2 implementation and Heroku conventions as inspiration.
Docs: