-
Notifications
You must be signed in to change notification settings - Fork 486
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
Module with declare and import #5968
Closed
Closed
Changes from 2 commits
Commits
Show all changes
41 commits
Select commit
Hold shift + click to select a range
a73e2e3
Implement new modules with import and declare keywords.
wildum ebc9028
Merge branch 'main' into modules-declare-import
wildum aca2289
minor changes following review
wildum 8f50ca1
use Declare type instead of plain string to use the AST instead of pa…
wildum 69a1d69
introduces a new componentNode interface
wildum f020082
rename according to new terminology
wildum 130a98e
some additional renaming
wildum c883d1a
introduce the component node manager
wildum cb983c5
use deprecation notice on module component constructor
wildum e7fb05e
fix test after changing error message
wildum e2f9c99
custom component should return the managed component via Component()
wildum 3c24a3c
introduces LoaderConfigOptions used to pass options like additional d…
wildum b7aa47e
ignore linter warning on deprecated func
wildum 189a2b2
add deprecated doc on module Exports type
wildum 7bfc196
rename function that decides if a component is a custom one
wildum 1cb8a31
rename firstPart to namespace
wildum b93184e
make declare fields private
wildum d40e409
add comment regarding the redundancy in the Declare struct
wildum ce8ec22
rename blocks in loader to prevent confusion between componentBlocks …
wildum 28d8f5f
remove unused field in loader
wildum 3a9d20d
fix mutex and flaky test
wildum 32e7811
remove unecessary mutex in declare
wildum fc0770e
add more doc for the Apply function
wildum e0e018b
rename and document interfaces
wildum 3ca3692
cleanup on comments, namings and mutex
wildum 40f5fd9
update running children in case of an update of the content in import…
wildum 79b98ea
check for go routines leak
wildum 65de374
comment cleanup
wildum 219474a
can cancel on all paths for linter
wildum 4f2d64d
improve import children handling
wildum 3fdc372
renaming
wildum d53e29e
optimization to avoid reloading a module if the imported content did …
wildum f384f0b
improve tests
wildum fdcf18a
remove forgotten print
wildum d8e37c0
merge main
wildum c0bfd61
Merge branch 'main' into modules-declare-import
wildum fea7a8a
store args in custom component
wildum dbc4be7
merge main
wildum 5ef4670
refactor integration tests to remove redundancy
wildum dcbc6d6
merge main
wildum 60d6eca
cleanup
wildum 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
There are no files selected for viewing
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
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
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
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
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
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
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
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
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
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
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,21 @@ | ||
declare "myModule" { | ||
argument "scrape_endpoint" {} | ||
|
||
argument "forward_to" {} | ||
|
||
argument "scrape_interval" { | ||
optional = true | ||
default = "1s" | ||
} | ||
|
||
prometheus.scrape "scrape_prom_metrics_module_file" { | ||
targets = [ | ||
{"__address__" = argument.scrape_endpoint.value}, | ||
] | ||
forward_to = argument.forward_to.value | ||
scrape_classic_histograms = true | ||
enable_protobuf_negotiation = true | ||
scrape_interval = argument.scrape_interval.value | ||
scrape_timeout = "500ms" | ||
} | ||
} |
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
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,38 @@ | ||
import.file "import_loki" { | ||
filename = "loki.river" | ||
} | ||
|
||
import_loki.loki "loki" {} | ||
|
||
import.file "import_prom_scrape" { | ||
filename = "prom-scrape.river" | ||
} | ||
|
||
declare "target" { | ||
export "output" { | ||
value = "localhost:9001" | ||
} | ||
} | ||
|
||
target "promTarget" {} | ||
|
||
import_prom_scrape.scrape "promScraper" { | ||
scrape_endpoint = target.promTarget.output | ||
forward_to = [prometheus.remote_write.module_file.receiver] | ||
} | ||
|
||
prometheus.remote_write "module_file" { | ||
endpoint { | ||
url = "http://localhost:9009/api/v1/push" | ||
send_native_histograms = true | ||
metadata_config { | ||
send_interval = "1s" | ||
} | ||
queue_config { | ||
max_samples_per_send = 100 | ||
} | ||
} | ||
external_labels = { | ||
test_name = "module_file", | ||
} | ||
} |
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,5 @@ | ||
declare "getLogFile" { | ||
export "output" { | ||
value = [{__path__ = "logs.txt"}] | ||
} | ||
} |
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,13 @@ | ||
[2023-10-02 14:25:43] INFO: Starting the web application... | ||
[2023-10-02 14:25:45] DEBUG: Database connection established. | ||
[2023-10-02 14:26:01] INFO: User 'john_doe' logged in. | ||
[2023-10-02 14:26:05] WARNING: User 'john_doe' attempted to access restricted area. | ||
[2023-10-02 14:26:10] ERROR: Failed to retrieve data for item ID: 1234. | ||
[2023-10-02 14:26:15] INFO: User 'john_doe' logged out. | ||
[2023-10-02 14:27:00] INFO: User 'admin' logged in. | ||
[2023-10-02 14:27:05] INFO: Data backup started. | ||
[2023-10-02 14:30:00] INFO: Data backup completed successfully. | ||
[2023-10-02 14:31:23] ERROR: Database connection lost. Retrying in 5 seconds... | ||
[2023-10-02 14:31:28] INFO: Database reconnected. | ||
[2023-10-02 14:32:00] INFO: User 'admin' logged out. | ||
[2023-10-02 14:32:05] INFO: Shutting down the web application... |
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,17 @@ | ||
import.file "logTarget" { | ||
filename = "logfile.river" | ||
} | ||
|
||
import.file "write" { | ||
filename = "loki_write.river" | ||
} | ||
|
||
declare "loki" { | ||
logTarget.getLogFile "logFile" {} | ||
loki.source.file "test" { | ||
targets = logTarget.getLogFile.logFile.output | ||
forward_to = [write.loki_write.default.receiver] | ||
} | ||
write.loki_write "default" {} | ||
} | ||
|
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,13 @@ | ||
declare "loki_write" { | ||
loki.write "test" { | ||
endpoint { | ||
url = "http://localhost:3100/loki/api/v1/push" | ||
} | ||
external_labels = { | ||
test_name = "module_file", | ||
} | ||
} | ||
export "receiver" { | ||
value = loki.write.test.receiver | ||
} | ||
} |
Oops, something went wrong.
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.
Can we not remove this field? I think it's an important part of this API to be able to know what the arguments/exports types of a builtin component are, as this will come into play in the future when we explore plugins and other tooling around components.
Keeping this the same also means we'll have even more consistency between native and non-native components, which I find ideal.
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.
But the registration does not have the types? It has "Args Arguments" and "Exports Exports" which are already inside of the Info struct as "Arguments Arguments" and "Exports Exports". Removing the Registration field removes this repetition and the "Build" function (which should not be there because retrieving info about a particular instance of a component to create a new component of the same type should not be a correct way to create a component)
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.
The registration does have the types; it holds the zero value for the arguments and exports types of a given component, and it's used for unmarshaling. In the future, those fields could also be used for additional validity checking without instantiating components (e.g., #3844).
I agree, but I also didn't suggest that the registration from this API would be the way to construct new components; just for the full set of information associated with a component.
Let me think about this a bit. This is a public API, which I think we need to be cautious about changing. I need to consider what the future implications are for not including the registration in this API, particularly around whether that limits us with the web UI or any other future services.
We should change APIs deliberately and not to make another feature easier. So if we're going to change this API, we should make sure it's the right change.
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 builtin component, the args and exports are initialized by the registration when the node is created. That means that they also have the types and that we don't need the registration object.
Also registration is currently something specific for the builtin components. It is not used by the custom components so that would require some workaround to keep it in the component_provider info